[PATCH] netfs: Fix netfs_read_gaps() to use separate sink folios

[PATCH] netfs: Fix netfs_read_gaps() to use separate sink folios

[David Howells]

    
Fix netfs_read_gaps() to use separate folios rather than re-using a single
sink folio to discard the unwanted data so that cifs checksum checking sees
all the data that was fetched.

Fixes: 7f84a7b9892d ("netfs: Make netfs_read_folio() handle streaming-write pages")
Reported-by: Frank Sorenson <sorenson@redhat.com>
Closes: https://lore.kernel.org/r/a385053c-1c4a-4060-a3bb-befa007ddb33@redhat.com/
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Frank Sorenson <sorenson@redhat.com>
cc: Paulo Alcantara <pc@manguebit.org>
cc: Namjae Jeon <linkinjeon@kernel.org>
cc: netfs@lists.linux.dev
cc: linux-cifs@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---
 fs/netfs/buffered_read.c |   34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index 424df70a5c30..105194de6e13 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -482,15 +482,14 @@ static int netfs_read_gaps(struct file *file, struct folio *folio)
 	struct netfs_group *group = netfs_folio_group(folio);
 	struct netfs_folio *finfo = netfs_folio_info(folio);
 	struct netfs_inode *ctx = netfs_inode(mapping->host);
-	struct folio *sink = NULL;
-	struct bio_vec *bvec;
+	struct bio_vec *bvec = NULL;
 	unsigned int from = finfo->dirty_offset;
 	unsigned int to = from + finfo->dirty_len;
-	unsigned int off = 0, i = 0;
+	unsigned int off = 0;
 	size_t flen = folio_size(folio);
 	size_t nr_bvec = flen / PAGE_SIZE + 2;
 	size_t part;
-	int ret;
+	int ret, i = 0, sink_from = -1, sink_to = -1;
 
 	_enter("%lx", folio->index);
 
@@ -515,24 +514,23 @@ static int netfs_read_gaps(struct file *file, struct folio *folio)
 	if (!bvec)
 		goto discard;
 
-	sink = folio_alloc(GFP_KERNEL, 0);
-	if (!sink) {
-		kfree(bvec);
-		goto discard;
-	}
-
 	trace_netfs_folio(folio, netfs_folio_trace_read_gaps);
 
-	rreq->direct_bv = bvec;
-	rreq->direct_bv_count = nr_bvec;
 	if (from > 0) {
 		bvec_set_folio(&bvec[i++], folio, from, 0);
 		off = from;
 	}
+	sink_from = i;
 	while (off < to) {
+		struct folio *sink = folio_alloc(GFP_KERNEL, 0);
+
+		if (!sink)
+			goto discard;
 		part = min_t(size_t, to - off, PAGE_SIZE);
-		bvec_set_folio(&bvec[i++], sink, part, 0);
+		bvec_set_folio(&bvec[i], sink, part, 0);
 		off += part;
+		sink_to = i;
+		i++;
 	}
 	if (to < flen)
 		bvec_set_folio(&bvec[i++], folio, flen - to, to);
@@ -553,8 +551,10 @@ static int netfs_read_gaps(struct file *file, struct folio *folio)
 		folio_mark_uptodate(folio);
 	}
 
-	if (sink)
-		folio_put(sink);
+	if (sink_to >= 0)
+		for (; sink_from <= sink_to; sink_from++)
+			folio_put(bvec_folio(&bvec[sink_from]));
+	kfree(bvec);
 	folio_unlock(folio);
 	netfs_put_request(rreq, netfs_rreq_trace_put_return);
 	return ret < 0 ? ret : 0;
@@ -563,6 +563,10 @@ static int netfs_read_gaps(struct file *file, struct folio *folio)
 	netfs_put_failed_request(rreq);
 alloc_error:
 	folio_unlock(folio);
+	if (sink_to >= 0)
+		for (; sink_from <= sink_to; sink_from++)
+			folio_put(bvec_folio(&bvec[sink_from]));
+	kfree(bvec);
 	return ret;
 }
 

Re: [PATCH] netfs: Fix netfs_read_gaps() to use separate sink folios

[Paulo Alcantara]

David Howells <dhowells@redhat.com> writes:

>     
> Fix netfs_read_gaps() to use separate folios rather than re-using a single
> sink folio to discard the unwanted data so that cifs checksum checking sees
> all the data that was fetched.
>
> Fixes: 7f84a7b9892d ("netfs: Make netfs_read_folio() handle streaming-write pages")
> Reported-by: Frank Sorenson <sorenson@redhat.com>
> Closes: https://lore.kernel.org/r/a385053c-1c4a-4060-a3bb-befa007ddb33@redhat.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> Tested-by: Frank Sorenson <sorenson@redhat.com>
> cc: Paulo Alcantara <pc@manguebit.org>
> cc: Namjae Jeon <linkinjeon@kernel.org>
> cc: netfs@lists.linux.dev
> cc: linux-cifs@vger.kernel.org
> cc: linux-fsdevel@vger.kernel.org

Reviewed-by: Paulo Alcantara <pc@manguebit.org>