fuse: writepages: crop secondary requests

If writeback happens while fuse is in FUSE_NOWRITE condition, the request will be queued but not processed immediately (see fuse_flush_writepages()). Until FUSE_NOWRITE becomes relaxed, more writebacks can happen. They will be queued as "secondary" requests to that first ("primary") request. Existing implementation crops only primary request. This is not correct because a subsequent extending write(2) may increase i_size and then secondary requests won't be cropped properly. The result would be stale data written to the server to a file offset where zeros must be. Similar problem may happen if secondary requests are attached to an in-flight request that was already cropped. The patch solves the issue by cropping all secondary requests in fuse_writepage_end(). Thanks to Miklos for idea. Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

fuse: writepages: crop secondary requests
If writeback happens while fuse is in FUSE_NOWRITE condition, the request will be queued but not processed immediately (see fuse_flush_writepages()). Until FUSE_NOWRITE becomes relaxed, more writebacks can happen. They will be queued as "secondary" requests to that first ("primary") request. Existing implementation crops only primary request. This is not correct because a subsequent extending write(2) may increase i_size and then secondary requests won't be cropped properly. The result would be stale data written to the server to a file offset where zeros must be. Similar problem may happen if secondary requests are attached to an in-flight request that was already cropped. The patch solves the issue by cropping all secondary requests in fuse_writepage_end(). Thanks to Miklos for idea. Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Maxim Patlasov · Miklos Szeredi
1 parent f6011081f5
Showing 1 changed file with 31 additions and 5 deletions Side-by-side Diff
fs/fuse/file.c
@@ -1436,12 +1436,12 @@
 }
  
 /* Called under fc->lock, may release and reacquire it */
-static void fuse_send_writepage(struct fuse_conn *fc, struct fuse_req *req)
+static void fuse_send_writepage(struct fuse_conn *fc, struct fuse_req *req,
+				loff_t size)
 __releases(fc->lock)
 __acquires(fc->lock)
 {
 	struct fuse_inode *fi = get_fuse_inode(req->inode);
-	loff_t size = i_size_read(req->inode);
 	struct fuse_write_in *inarg = &req->misc.write.in;
 	__u64 data_size = req->num_pages * PAGE_CACHE_SIZE;
  
  
@@ -1482,12 +1482,13 @@
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
+	size_t crop = i_size_read(inode);
 	struct fuse_req *req;
  
 	while (fi->writectr >= 0 && !list_empty(&fi->queued_writes)) {
 		req = list_entry(fi->queued_writes.next, struct fuse_req, list);
 		list_del_init(&req->list);
-		fuse_send_writepage(fc, req);
+		fuse_send_writepage(fc, req, crop);
 	}
 }
  
  
@@ -1499,12 +1500,37 @@
 	mapping_set_error(inode->i_mapping, req->out.h.error);
 	spin_lock(&fc->lock);
 	while (req->misc.write.next) {
+		struct fuse_conn *fc = get_fuse_conn(inode);
+		struct fuse_write_in *inarg = &req->misc.write.in;
 		struct fuse_req *next = req->misc.write.next;
 		req->misc.write.next = next->misc.write.next;
 		next->misc.write.next = NULL;
 		list_add(&next->writepages_entry, &fi->writepages);
-		list_add_tail(&next->list, &fi->queued_writes);
-		fuse_flush_writepages(inode);
+
+		/*
+		 * Skip fuse_flush_writepages() to make it easy to crop requests
+		 * based on primary request size.
+		 *
+		 * 1st case (trivial): there are no concurrent activities using
+		 * fuse_set/release_nowrite.  Then we're on safe side because
+		 * fuse_flush_writepages() would call fuse_send_writepage()
+		 * anyway.
+		 *
+		 * 2nd case: someone called fuse_set_nowrite and it is waiting
+		 * now for completion of all in-flight requests.  This happens
+		 * rarely and no more than once per page, so this should be
+		 * okay.
+		 *
+		 * 3rd case: someone (e.g. fuse_do_setattr()) is in the middle
+		 * of fuse_set_nowrite..fuse_release_nowrite section.  The fact
+		 * that fuse_set_nowrite returned implies that all in-flight
+		 * requests were completed along with all of their secondary
+		 * requests.  Further primary requests are blocked by negative
+		 * writectr.  Hence there cannot be any in-flight requests and
+		 * no invocations of fuse_writepage_end() while we're in
+		 * fuse_set_nowrite..fuse_release_nowrite section.
+		 */
+		fuse_send_writepage(fc, next, inarg->offset + inarg->size);
 	}
 	fi->writectr--;
 	fuse_writepage_finish(fc, req);
...	...	@@ -1436,12 +1436,12 @@
1436	1436	}
1437	1437
1438	1438	/* Called under fc->lock, may release and reacquire it */
1439		-static void fuse_send_writepage(struct fuse_conn fc, struct fuse_req req)
	1439	+static void fuse_send_writepage(struct fuse_conn fc, struct fuse_req req,
	1440	+ loff_t size)
1440	1441	__releases(fc->lock)
1441	1442	__acquires(fc->lock)
1442	1443	{
1443	1444	struct fuse_inode *fi = get_fuse_inode(req->inode);
1444		- loff_t size = i_size_read(req->inode);
1445	1445	struct fuse_write_in *inarg = &req->misc.write.in;
1446	1446	__u64 data_size = req->num_pages * PAGE_CACHE_SIZE;
1447	1447
1448	1448
...	...	@@ -1482,12 +1482,13 @@
1482	1482	{
1483	1483	struct fuse_conn *fc = get_fuse_conn(inode);
1484	1484	struct fuse_inode *fi = get_fuse_inode(inode);
	1485	+ size_t crop = i_size_read(inode);
1485	1486	struct fuse_req *req;
1486	1487
1487	1488	while (fi->writectr >= 0 && !list_empty(&fi->queued_writes)) {
1488	1489	req = list_entry(fi->queued_writes.next, struct fuse_req, list);
1489	1490	list_del_init(&req->list);
1490		- fuse_send_writepage(fc, req);
	1491	+ fuse_send_writepage(fc, req, crop);
1491	1492	}
1492	1493	}
1493	1494
1494	1495
...	...	@@ -1499,12 +1500,37 @@
1499	1500	mapping_set_error(inode->i_mapping, req->out.h.error);
1500	1501	spin_lock(&fc->lock);
1501	1502	while (req->misc.write.next) {
	1503	+ struct fuse_conn *fc = get_fuse_conn(inode);
	1504	+ struct fuse_write_in *inarg = &req->misc.write.in;
1502	1505	struct fuse_req *next = req->misc.write.next;
1503	1506	req->misc.write.next = next->misc.write.next;
1504	1507	next->misc.write.next = NULL;
1505	1508	list_add(&next->writepages_entry, &fi->writepages);
1506		- list_add_tail(&next->list, &fi->queued_writes);
1507		- fuse_flush_writepages(inode);
	1509	+
	1510	+ /*
	1511	+ * Skip fuse_flush_writepages() to make it easy to crop requests
	1512	+ * based on primary request size.
	1513	+ *
	1514	+ * 1st case (trivial): there are no concurrent activities using
	1515	+ * fuse_set/release_nowrite. Then we're on safe side because
	1516	+ * fuse_flush_writepages() would call fuse_send_writepage()
	1517	+ * anyway.
	1518	+ *
	1519	+ * 2nd case: someone called fuse_set_nowrite and it is waiting
	1520	+ * now for completion of all in-flight requests. This happens
	1521	+ * rarely and no more than once per page, so this should be
	1522	+ * okay.
	1523	+ *
	1524	+ * 3rd case: someone (e.g. fuse_do_setattr()) is in the middle
	1525	+ * of fuse_set_nowrite..fuse_release_nowrite section. The fact
	1526	+ * that fuse_set_nowrite returned implies that all in-flight
	1527	+ * requests were completed along with all of their secondary
	1528	+ * requests. Further primary requests are blocked by negative
	1529	+ * writectr. Hence there cannot be any in-flight requests and
	1530	+ * no invocations of fuse_writepage_end() while we're in
	1531	+ * fuse_set_nowrite..fuse_release_nowrite section.
	1532	+ */
	1533	+ fuse_send_writepage(fc, next, inarg->offset + inarg->size);
1508	1534	}
1509	1535	fi->writectr--;
1510	1536	fuse_writepage_finish(fc, req);