fuse: re-validate the DLM grant after waiting on the coherency gate

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);
 		}
 	}
 

fs/fuse/fuse_dlm_cache.c

@@ -493,15 +493,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 +548,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,21 +570,26 @@ 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) {
 			/* fuse server is seriously broken */
 			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);
 		}
 }

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 */

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);
 		}