From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: David Howells <dhowells@redhat.com>,
Shyam Prasad N <nspmangalore@gmail.com>,
Steve French <sfrench@samba.org>,
Paulo Alcantara <pc@manguebit.com>,
Jeff Layton <jlayton@kernel.org>,
linux-cifs@vger.kernel.org, netfs@lists.linux.dev,
linux-fsdevel@vger.kernel.org,
Christian Brauner <brauner@kernel.org>,
Sasha Levin <sashal@kernel.org>
Subject: [PATCH AUTOSEL 6.12 07/20] netfs: Fix non-contiguous donation between completed reads
Date: Mon, 13 Jan 2025 13:34:12 -0500 [thread overview]
Message-ID: <20250113183425.1783715-7-sashal@kernel.org> (raw)
In-Reply-To: <20250113183425.1783715-1-sashal@kernel.org>
From: David Howells <dhowells@redhat.com>
[ Upstream commit c8b90d40d5bba8e6fba457b8a7c10d3c0d467e37 ]
When a read subrequest finishes, if it doesn't have sufficient coverage to
complete the folio(s) covering either side of it, it will donate the excess
coverage to the adjacent subrequests on either side, offloading
responsibility for unlocking the folio(s) covered to them.
Now, preference is given to donating down to a lower file offset over
donating up because that check is done first - but there's no check that
the lower subreq is actually contiguous, and so we can end up donating
incorrectly.
The scenario seen[1] is that an 8MiB readahead request spanning four 2MiB
folios is split into eight 1MiB subreqs (numbered 1 through 8). These
terminate in the order 1,6,2,5,3,7,4,8. What happens is:
- 1 donates to 2
- 6 donates to 5
- 2 completes, unlocking the first folio (with 1).
- 5 completes, unlocking the third folio (with 6).
- 3 donates to 4
- 7 donates to 4 incorrectly
- 4 completes, unlocking the second folio (with 3), but can't use
the excess from 7.
- 8 donates to 4, also incorrectly.
Fix this by preventing downward donation if the subreqs are not contiguous
(in the example above, 7 donates to 4 across the gap left by 5 and 6).
Reported-by: Shyam Prasad N <nspmangalore@gmail.com>
Closes: https://lore.kernel.org/r/CANT5p=qBwjBm-D8soFVVtswGEfmMtQXVW83=TNfUtvyHeFQZBA@mail.gmail.com/
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://lore.kernel.org/r/526707.1733224486@warthog.procyon.org.uk/ [1]
Link: https://lore.kernel.org/r/20241213135013.2964079-3-dhowells@redhat.com
cc: Steve French <sfrench@samba.org>
cc: Paulo Alcantara <pc@manguebit.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/netfs/read_collect.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index 3cbb289535a8..b415e3972336 100644
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -247,16 +247,17 @@ static bool netfs_consume_read_data(struct netfs_io_subrequest *subreq, bool was
/* Deal with the trickiest case: that this subreq is in the middle of a
* folio, not touching either edge, but finishes first. In such a
- * case, we donate to the previous subreq, if there is one, so that the
- * donation is only handled when that completes - and remove this
- * subreq from the list.
+ * case, we donate to the previous subreq, if there is one and if it is
+ * contiguous, so that the donation is only handled when that completes
+ * - and remove this subreq from the list.
*
* If the previous subreq finished first, we will have acquired their
* donation and should be able to unlock folios and/or donate nextwards.
*/
if (!subreq->consumed &&
!prev_donated &&
- !list_is_first(&subreq->rreq_link, &rreq->subrequests)) {
+ !list_is_first(&subreq->rreq_link, &rreq->subrequests) &&
+ subreq->start == prev->start + prev->len) {
prev = list_prev_entry(subreq, rreq_link);
WRITE_ONCE(prev->next_donated, prev->next_donated + subreq->len);
subreq->start += subreq->len;
--
2.39.5
next prev parent reply other threads:[~2025-01-13 18:34 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-13 18:34 [PATCH AUTOSEL 6.12 01/20] mac802154: check local interfaces before deleting sdata list Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 02/20] hfs: Sanity check the root record Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 03/20] fs/qnx6: Fix building with GCC 15 Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 04/20] iomap: pass byte granular end position to iomap_add_to_ioend Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 05/20] fs: fix missing declaration of init_files Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 06/20] kheaders: Ignore silly-rename files Sasha Levin
2025-01-13 18:34 ` Sasha Levin [this message]
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 08/20] cachefiles: Parse the "secctx" immediately Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 09/20] scsi: ufs: core: Honor runtime/system PM levels if set by host controller drivers Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 10/20] gpio: virtuser: lock up configfs that an instantiated device depends on Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 11/20] gpio: sim: " Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 12/20] selftests: tc-testing: reduce rshift value Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 13/20] ovl: pass realinode to ovl_encode_real_fh() instead of realdentry Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 14/20] platform/x86/intel: power-domains: Add Clearwater Forest support Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 15/20] platform/x86: ISST: Add Clearwater Forest to support list Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 16/20] ACPI: resource: acpi_dev_irq_override(): Check DMI match last Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 17/20] sched_ext: keep running prev when prev->scx.slice != 0 Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 18/20] iomap: avoid avoid truncating 64-bit offset to 32 bits Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 19/20] afs: Fix merge preference rule failure condition Sasha Levin
2025-01-13 18:34 ` [PATCH AUTOSEL 6.12 20/20] poll_wait: add mb() to fix theoretical race between waitqueue_active() and .poll() Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250113183425.1783715-7-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=brauner@kernel.org \
--cc=dhowells@redhat.com \
--cc=jlayton@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netfs@lists.linux.dev \
--cc=nspmangalore@gmail.com \
--cc=pc@manguebit.com \
--cc=sfrench@samba.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox