diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 91faa406921057..ca1f4059455d66 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1298,10 +1298,35 @@ static ssize_t fuse_cache_read_iter(struct kiocb *iocb, struct iov_iter *to) * wb_sem is NULL on non-writeback+dlm mounts (gate inactive). */ if (wb_sem) { + int lock_err = 0; + percpu_down_read(wb_sem); - if (fuse_inode_force_dio(inode)) { + for (;;) { + if (fuse_inode_force_dio(inode)) { + percpu_up_read(wb_sem); + return fuse_direct_read_iter(iocb, to); + } + /* + * The DLM lock was requested before entering the gate, + * and the NOTIFY invalidate we may just have waited on + * revokes locks under the gate write side. Re-check + * the grant here and re-request with the gate dropped, + * so a FUSE_DLM_WB_LOCK round trip never parks a + * pending invalidate behind our own gate hold. Once + * the check passes the lock cannot go away for the + * rest of the gate hold. A failed request falls + * through unlocked, as before. + */ + if (lock_err || !fc->dlm || + fuse_dlm_lock_is_held(fi, iocb->ki_pos, + iov_iter_count(to), + FUSE_PAGE_LOCK_READ)) + break; percpu_up_read(wb_sem); - return fuse_direct_read_iter(iocb, to); + lock_err = fuse_get_dlm_lock(file, iocb->ki_pos, + iov_iter_count(to), + FUSE_PAGE_LOCK_READ); + percpu_down_read(wb_sem); } } @@ -1821,6 +1846,8 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from) bool writeback = false; bool wb_guard = false; bool exclusive = true; + loff_t dlm_pos = 0; + size_t dlm_len = 0; /* * The inode may have been latched into forced direct IO -- by a @@ -1840,33 +1867,8 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from) return err; if (!fc->handle_killpriv_v2 || - !setattr_should_drop_suidgid(idmap, file_inode(file))) { + !setattr_should_drop_suidgid(idmap, file_inode(file))) writeback = true; - - /* - * If we have dlm support acquire the lock for the area - * we are writing into. - * dlm lock is only needed as the write is cached and the - * fuse server is not notified otherwise - */ - if (fc->dlm) { - /* - * Note that a file opened with O_APPEND will have - * relative values in ki_pos. This code is here for - * convenience and for libfuse overlay test. - * Filesystems should handle O_APPEND with 'direct io' - * to additionally get the performance benefits of - * 'parallel direct writes'. - */ - loff_t pos = file->f_flags & O_APPEND ? - i_size_read(inode) + iocb->ki_pos : - iocb->ki_pos; - size_t length = iov_iter_count(from); - - fuse_get_dlm_lock(file, pos, length, - FUSE_PAGE_LOCK_WRITE); - } - } } exclusive = fuse_cache_wr_exclusive_lock(iocb, writeback); @@ -1875,6 +1877,33 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from) else inode_lock_shared(inode); + /* + * If we have dlm support acquire the lock for the area we are + * writing into. The dlm lock is only needed as the write is cached + * and the fuse server is not notified otherwise. Requested under + * the inode lock so the sleep on a contended i_rwsem cannot sit + * between grant and use -- a NOTIFY invalidate arriving in that + * window revokes the lock -- and so the O_APPEND position below is + * computed against a stable i_size. + */ + if (writeback && fc->dlm) { + /* + * Note that a file opened with O_APPEND will have + * relative values in ki_pos. This code is here for + * convenience and for libfuse overlay test. + * Filesystems should handle O_APPEND with 'direct io' + * to additionally get the performance benefits of + * 'parallel direct writes'. + */ + dlm_pos = file->f_flags & O_APPEND ? + i_size_read(inode) + iocb->ki_pos : + iocb->ki_pos; + dlm_len = iov_iter_count(from); + + fuse_get_dlm_lock(file, dlm_pos, dlm_len, + FUSE_PAGE_LOCK_WRITE); + } + /* * The forced-direct-IO latch feature is active under writeback+dlm; * hold the coherency gate (wb_inval_rwsem) for read across the @@ -1885,15 +1914,30 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from) * lock) and re-route to the direct path if set. Taken before * task_io_account_write() so a re-route is not double-counted; the DLM * write lock taken above is harmless as the direct path does its own - * server coordination. + * server coordination. Also re-validate that DLM grant: a NOTIFY + * that held the gate while we waited revokes locks under the write + * side, so re-request with the gate dropped until the grant survives + * into the gate hold (see fuse_cache_read_iter()). */ wb_guard = !!wb_sem; if (wb_guard) { + int lock_err = 0; + percpu_down_read(wb_sem); - if (fuse_inode_force_dio(inode)) { + for (;;) { + if (fuse_inode_force_dio(inode)) { + percpu_up_read(wb_sem); + fuse_cache_wr_unlock(inode, exclusive); + return fuse_direct_write_iter(iocb, from); + } + if (lock_err || !writeback || !fc->dlm || + fuse_dlm_lock_is_held(fi, dlm_pos, dlm_len, + FUSE_PAGE_LOCK_WRITE)) + break; percpu_up_read(wb_sem); - fuse_cache_wr_unlock(inode, exclusive); - return fuse_direct_write_iter(iocb, from); + lock_err = fuse_get_dlm_lock(file, dlm_pos, dlm_len, + FUSE_PAGE_LOCK_WRITE); + percpu_down_read(wb_sem); } } diff --git a/fs/fuse/fuse_dlm_cache.c b/fs/fuse/fuse_dlm_cache.c index 40eda6daf75cae..50daf1e8ca8183 100644 --- a/fs/fuse/fuse_dlm_cache.c +++ b/fs/fuse/fuse_dlm_cache.c @@ -120,26 +120,25 @@ static void fuse_dlm_try_merge(struct fuse_dlm_cache *cache, uint64_t start, uint64_t end) { struct fuse_dlm_range *range, *next; - struct rb_node *node; + uint64_t first = start ? start - 1 : start; + uint64_t last = end < U64_MAX ? end + 1 : end; if (!cache) return; - /* Find the first range that might need merging */ - range = NULL; - node = rb_first_cached(&cache->ranges); - while (node) { - range = rb_entry(node, struct fuse_dlm_range, rb); - if (range->end >= start - 1) - break; - node = rb_next(node); - } - - if (!range || range->start > end + 1) - return; + /* + * Find the first range that might need merging. Directly adjacent + * ranges can merge, hence the region is widened by one unit to each + * side (saturating at the type bounds). This must stay an + * interval-tree lookup: the tree holds every cached grant of the + * inode and strided writers grow it for the lifetime of the file, + * so seeding the merge by walking from the tree minimum would make + * every new grant cost a full scan. + */ + range = fuse_page_it_iter_first(&cache->ranges, first, last); /* Try to merge ranges in and around the specified region */ - while (range && range->start <= end + 1) { + while (range && range->start <= last) { /* Get next range before we potentially modify the tree */ next = NULL; if (rb_next(&range->rb)) { @@ -150,11 +149,11 @@ static void fuse_dlm_try_merge(struct fuse_dlm_cache *cache, uint64_t start, /* Try to merge with next range if adjacent and same mode */ if (next && range->mode == next->mode && range->end + 1 == next->start) { - /* Merge ranges */ - range->end = next->end; - - /* Remove next from tree */ + /* Merge ranges: re-insert so __subtree_end is updated */ fuse_page_it_remove(next, &cache->ranges); + fuse_page_it_remove(range, &cache->ranges); + range->end = next->end; + fuse_page_it_insert(range, &cache->ranges); kfree(next); /* Continue with the same range */ @@ -188,6 +187,7 @@ int fuse_dlm_lock_range(struct fuse_inode *inode, uint64_t start, struct fuse_dlm_cache *cache = &inode->dlm_locked_areas; struct fuse_dlm_range *range, *new_range, *next; int lock_mode; + bool covered_to_end = false; int ret = 0; LIST_HEAD(to_lock); LIST_HEAD(to_upgrade); @@ -233,14 +233,17 @@ int fuse_dlm_lock_range(struct fuse_inode *inode, uint64_t start, } /* Move current_start past this range */ - current_start = max(current_start, range->end + 1); + if (range->end >= end) + covered_to_end = true; + else + current_start = max(current_start, range->end + 1); /* Move to next range */ range = next; } /* If there's a gap after the last range to the end, extend the range */ - if (current_start <= end) { + if (!covered_to_end && current_start <= end) { new_range = kmalloc(sizeof(*new_range), GFP_KERNEL); if (!new_range) { ret = -ENOMEM; @@ -322,13 +325,17 @@ static int fuse_dlm_punch_hole(struct fuse_dlm_cache *cache, uint64_t start, /* If the hole is at the beginning of the range */ if (start == range->start) { + fuse_page_it_remove(range, &cache->ranges); range->start = end + 1; + fuse_page_it_insert(range, &cache->ranges); goto out; } /* If the hole is at the end of the range */ if (end == range->end) { + fuse_page_it_remove(range, &cache->ranges); range->end = start - 1; + fuse_page_it_insert(range, &cache->ranges); goto out; } @@ -400,10 +407,14 @@ int fuse_dlm_unlock_range(struct fuse_inode *inode, break; } else if (start > range->start) { /* Adjust the end of the range */ + fuse_page_it_remove(range, &cache->ranges); range->end = start - 1; + fuse_page_it_insert(range, &cache->ranges); } else if (end < range->end) { /* Adjust the start of the range */ + fuse_page_it_remove(range, &cache->ranges); range->start = end + 1; + fuse_page_it_insert(range, &cache->ranges); } else { /* Complete overlap, remove the range */ fuse_page_it_remove(range, &cache->ranges); @@ -475,6 +486,12 @@ bool fuse_dlm_range_is_locked(struct fuse_inode *inode, uint64_t start, return false; } + /* Covered through the end of the requested range? */ + if (range->end >= end) { + up_read(&cache->lock); + return true; + } + /* Move current_start past this range */ current_start = range->end + 1; @@ -493,15 +510,38 @@ bool fuse_dlm_range_is_locked(struct fuse_inode *inode, uint64_t start, return true; } +/** + * fuse_dlm_lock_is_held - check that a byte range is covered by a granted lock + * @fi: the fuse inode + * @offset: byte offset into the file (need not be page-aligned) + * @length: length of the region in bytes (need not be page-aligned) + * @mode: FUSE_PAGE_LOCK_READ or FUSE_PAGE_LOCK_WRITE + * + * Re-validation helper for fuse_get_dlm_lock() callers: checks the same + * page-aligned range a fuse_get_dlm_lock() call with these arguments + * requests, against the live lock tree. + */ +bool fuse_dlm_lock_is_held(struct fuse_inode *fi, loff_t offset, + size_t length, enum fuse_page_lock_mode mode) +{ + uint64_t end = (offset + length - 1) | (PAGE_SIZE - 1); + + return fuse_dlm_range_is_locked(fi, offset & PAGE_MASK, end, mode); +} + /** * fuse_get_dlm_lock - request a dlm lock from the fuse server * @file: the file being accessed * @offset: byte offset into the file (need not be page-aligned) * @length: length of the region in bytes (need not be page-aligned) * @mode: FUSE_PAGE_LOCK_READ or FUSE_PAGE_LOCK_WRITE + * + * Return: 0 when the range is covered by a recorded grant on return, + * a negative error code otherwise. Callers re-validating the grant + * must not re-request on error or they would spin. */ -void fuse_get_dlm_lock(struct file *file, loff_t offset, - size_t length, enum fuse_page_lock_mode mode) +int fuse_get_dlm_lock(struct file *file, loff_t offset, + size_t length, enum fuse_page_lock_mode mode) { struct fuse_file *ff = file->private_data; struct inode *inode = file_inode(file); @@ -525,7 +565,7 @@ void fuse_get_dlm_lock(struct file *file, loff_t offset, * since a DLM implementation in the FUSE server should take care * of any races in lock requests */ if (fuse_dlm_range_is_locked(fi, offset, end, mode)) - return; /* we already have this area locked */ + return 0; /* we already have this area locked */ memset(&inarg, 0, sizeof(inarg)); inarg.fh = ff->fh; @@ -547,11 +587,11 @@ void fuse_get_dlm_lock(struct file *file, loff_t offset, if (err == -ENOSYS) { /* fuse server does not support dlm, save the info */ fc->dlm = 0; - return; + return err; } if (err) - return; + return err; else if (inarg.start < outarg.start || inarg.end > outarg.end) { @@ -559,9 +599,14 @@ void fuse_get_dlm_lock(struct file *file, loff_t offset, pr_warn("fuse: dlm lock request for %llu:%llu returned %llu:%llu bytes\n", inarg.start, inarg.end, outarg.start, outarg.end); fuse_abort_conn(fc); - return; + return -EIO; } else { - /* ignore any errors here, there is no way we can react appropriately */ - fuse_dlm_lock_range(fi, outarg.start, outarg.end, mode); + /* + * A failure to record the grant leaves it invisible + * to fuse_dlm_lock_is_held(); report it so callers + * fall through unlocked instead of re-requesting. + */ + return fuse_dlm_lock_range(fi, outarg.start, + outarg.end, mode); } } diff --git a/fs/fuse/fuse_dlm_cache.h b/fs/fuse/fuse_dlm_cache.h index 5c3deaa3536866..eb6edb3d8685ad 100644 --- a/fs/fuse/fuse_dlm_cache.h +++ b/fs/fuse/fuse_dlm_cache.h @@ -43,8 +43,12 @@ int fuse_dlm_unlock_range(struct fuse_inode *inode, uint64_t start, bool fuse_dlm_range_is_locked(struct fuse_inode *inode, uint64_t start, uint64_t end, enum fuse_page_lock_mode mode); +/* Re-validate a fuse_get_dlm_lock() grant against the live lock tree */ +bool fuse_dlm_lock_is_held(struct fuse_inode *inode, loff_t offset, + size_t length, enum fuse_page_lock_mode mode); + /* This is the interface to the filesystem */ -void fuse_get_dlm_lock(struct file *file, loff_t offset, - size_t length, enum fuse_page_lock_mode mode); +int fuse_get_dlm_lock(struct file *file, loff_t offset, + size_t length, enum fuse_page_lock_mode mode); #endif /* _FS_FUSE_DLM_CACHE_H */ diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index a81334a5f3b092..ff803836a0c4b0 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -858,16 +858,6 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, else pg_end = (offset + len - 1) >> PAGE_SHIFT; - if (fc->dlm && fc->writeback_cache) - /* Invalidate the range exactly as the fuse server requested - * except for the case where it sends -1. - * Note that this can lead to some inconsistencies if - * the fuse server sends unaligned data */ - fuse_dlm_unlock_range(fi, - offset, - pg_end == -1 ? 0 : - (offset + len - 1)); - /* * A data invalidation means another (remote) entity is modifying * the file. Two things happen here: @@ -923,6 +913,23 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, */ percpu_down_write(wb_sem); + /* + * Revoke the DLM lock range under the gate write + * side, atomically with the page drop: gate readers + * re-validate their grant right after entering, and + * a grant that passed that check must stay visible + * for their whole gate hold. + * The range is exactly what the fuse server + * requested except for the case where it sends -1. + * Note that this can lead to some inconsistencies + * if the fuse server sends unaligned data. + */ + if (fc->dlm && fc->writeback_cache) + fuse_dlm_unlock_range(fi, + offset, + pg_end == -1 ? 0 : + (offset + len - 1)); + if (hot && has_writer && !fuse_inode_force_dio(inode)) { spin_lock(&fi->lock); @@ -951,6 +958,14 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, pr_info_ratelimited("FUSE: inode %llu latched to direct IO on invalidation notify storm\n", nodeid); } else { + /* No gate on this inode (mmapped, DAX, backing or + * non-regular): drop the lock range unserialized, + * as before. */ + if (fc->dlm && fc->writeback_cache) + fuse_dlm_unlock_range(fi, + offset, + pg_end == -1 ? 0 : + (offset + len - 1)); invalidate_inode_pages2_range(inode->i_mapping, pg_start, pg_end); }