* [PATCH 1/4] netfs: Fix collection of results during pause when collection offloaded
2025-03-14 16:41 [PATCH 0/4] netfs: Miscellaneous fixes David Howells
@ 2025-03-14 16:41 ` David Howells
2025-03-14 16:41 ` [PATCH 2/4] netfs: Call `invalidate_cache` only if implemented David Howells
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2025-03-14 16:41 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Max Kellermann, Jeff Layton, netfs, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel,
linux-kernel, Steve French, Paulo Alcantara
A netfs read request can run in one of two modes: for synchronous reads
writes, the app thread does the collection of results and for asynchronous
reads, this is offloaded to a worker thread. This is controlled by the
NETFS_RREQ_OFFLOAD_COLLECTION flag.
Now, if a subrequest incurs an error, the NETFS_RREQ_PAUSE flag is set to
stop the issuing loop temporarily from issuing more subrequests until a
retry is successful or the request is abandoned.
When the issuing loop sees NETFS_RREQ_PAUSE, it jumps to
netfs_wait_for_pause() which will wait for the PAUSE flag to be cleared -
and whilst it is waiting, it will call out to the collector as more results
acrue... But this is the wrong thing to do if OFFLOAD_COLLECTION is set as
we can then end up with both the app thread and the work item collecting
results simultaneously.
This manifests itself occasionally when running the generic/323 xfstest
against multichannel cifs as an oops that's a bit random but frequently
involving io_submit() (the test does lots of simultaneous async DIO reads).
Fix this by only doing the collection in netfs_wait_for_pause() if the
NETFS_RREQ_OFFLOAD_COLLECTION is not set.
Fixes: e2d46f2ec332 ("netfs: Change the read result collector to only use one work item")
Reported-by: Steve French <stfrench@microsoft.com>
Signed-off-by: David Howells <dhowells@redhat.com>
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
---
fs/netfs/read_collect.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index 636cc5a98ef5..23c75755ad4e 100644
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -682,14 +682,16 @@ void netfs_wait_for_pause(struct netfs_io_request *rreq)
trace_netfs_rreq(rreq, netfs_rreq_trace_wait_queue);
prepare_to_wait(&rreq->waitq, &myself, TASK_UNINTERRUPTIBLE);
- subreq = list_first_entry_or_null(&stream->subrequests,
- struct netfs_io_subrequest, rreq_link);
- if (subreq &&
- (!test_bit(NETFS_SREQ_IN_PROGRESS, &subreq->flags) ||
- test_bit(NETFS_SREQ_MADE_PROGRESS, &subreq->flags))) {
- __set_current_state(TASK_RUNNING);
- netfs_read_collection(rreq);
- continue;
+ if (!test_bit(NETFS_RREQ_OFFLOAD_COLLECTION, &rreq->flags)) {
+ subreq = list_first_entry_or_null(&stream->subrequests,
+ struct netfs_io_subrequest, rreq_link);
+ if (subreq &&
+ (!test_bit(NETFS_SREQ_IN_PROGRESS, &subreq->flags) ||
+ test_bit(NETFS_SREQ_MADE_PROGRESS, &subreq->flags))) {
+ __set_current_state(TASK_RUNNING);
+ netfs_read_collection(rreq);
+ continue;
+ }
}
if (!test_bit(NETFS_RREQ_IN_PROGRESS, &rreq->flags) ||
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/4] netfs: Call `invalidate_cache` only if implemented
2025-03-14 16:41 [PATCH 0/4] netfs: Miscellaneous fixes David Howells
2025-03-14 16:41 ` [PATCH 1/4] netfs: Fix collection of results during pause when collection offloaded David Howells
@ 2025-03-14 16:41 ` David Howells
2025-03-14 16:41 ` [PATCH 3/4] netfs: Fix rolling_buffer_load_from_ra() to not clear mark bits David Howells
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2025-03-14 16:41 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Max Kellermann, Jeff Layton, netfs, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel,
linux-kernel, stable
From: Max Kellermann <max.kellermann@ionos.com>
Many filesystems such as NFS and Ceph do not implement the
`invalidate_cache` method. On those filesystems, if writing to the
cache (`NETFS_WRITE_TO_CACHE`) fails for some reason, the kernel
crashes like this:
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor instruction fetch in kernel mode
#PF: error_code(0x0010) - not-present page
PGD 0 P4D 0
Oops: Oops: 0010 [#1] SMP PTI
CPU: 9 UID: 0 PID: 3380 Comm: kworker/u193:11 Not tainted 6.13.3-cm4all1-hp #437
Hardware name: HP ProLiant DL380 Gen9/ProLiant DL380 Gen9, BIOS P89 10/17/2018
Workqueue: events_unbound netfs_write_collection_worker
RIP: 0010:0x0
Code: Unable to access opcode bytes at 0xffffffffffffffd6.
RSP: 0018:ffff9b86e2ca7dc0 EFLAGS: 00010202
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 7fffffffffffffff
RDX: 0000000000000001 RSI: ffff89259d576a18 RDI: ffff89259d576900
RBP: ffff89259d5769b0 R08: ffff9b86e2ca7d28 R09: 0000000000000002
R10: ffff89258ceaca80 R11: 0000000000000001 R12: 0000000000000020
R13: ffff893d158b9338 R14: ffff89259d576900 R15: ffff89259d5769b0
FS: 0000000000000000(0000) GS:ffff893c9fa40000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 000000054442e003 CR4: 00000000001706f0
Call Trace:
<TASK>
? __die+0x1f/0x60
? page_fault_oops+0x15c/0x460
? try_to_wake_up+0x2d2/0x530
? exc_page_fault+0x5e/0x100
? asm_exc_page_fault+0x22/0x30
netfs_write_collection_worker+0xe9f/0x12b0
? xs_poll_check_readable+0x3f/0x80
? xs_stream_data_receive_workfn+0x8d/0x110
process_one_work+0x134/0x2d0
worker_thread+0x299/0x3a0
? __pfx_worker_thread+0x10/0x10
kthread+0xba/0xe0
? __pfx_kthread+0x10/0x10
ret_from_fork+0x30/0x50
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Modules linked in:
CR2: 0000000000000000
This patch adds the missing `NULL` check.
Fixes: 0e0f2dfe880f ("netfs: Dispatch write requests to process a writeback slice")
Fixes: 288ace2f57c9 ("netfs: New writeback implementation")
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: netfs@lists.linux.dev
cc: linux-cifs@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
cc: stable@vger.kernel.org
---
fs/netfs/write_collect.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/netfs/write_collect.c b/fs/netfs/write_collect.c
index 294f67795f79..3fca59e6475d 100644
--- a/fs/netfs/write_collect.c
+++ b/fs/netfs/write_collect.c
@@ -400,7 +400,8 @@ void netfs_write_collection_worker(struct work_struct *work)
trace_netfs_rreq(wreq, netfs_rreq_trace_write_done);
if (wreq->io_streams[1].active &&
- wreq->io_streams[1].failed) {
+ wreq->io_streams[1].failed &&
+ ictx->ops->invalidate_cache) {
/* Cache write failure doesn't prevent writeback completion
* unless we're in disconnected mode.
*/
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] netfs: Fix rolling_buffer_load_from_ra() to not clear mark bits
2025-03-14 16:41 [PATCH 0/4] netfs: Miscellaneous fixes David Howells
2025-03-14 16:41 ` [PATCH 1/4] netfs: Fix collection of results during pause when collection offloaded David Howells
2025-03-14 16:41 ` [PATCH 2/4] netfs: Call `invalidate_cache` only if implemented David Howells
@ 2025-03-14 16:41 ` David Howells
2025-03-14 16:41 ` [PATCH 4/4] netfs: Fix netfs_unbuffered_read() to return ssize_t rather than int David Howells
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2025-03-14 16:41 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Max Kellermann, Jeff Layton, netfs, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel,
linux-kernel, Steve French, Paulo Alcantara
rolling_buffer_load_from_ra() looms large in the perf report because it
loops around doing an atomic clear for each of the three mark bits per
folio. However, this is both inefficient (it would be better to build a
mask and atomically AND them out) and unnecessary as they shouldn't be set.
Fix this by removing the loop.
Fixes: ee4cdf7ba857 ("netfs: Speed up buffered reading")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: Steve French <sfrench@samba.org>
cc: Paulo Alcantara <pc@manguebit.com>
cc: netfs@lists.linux.dev
cc: linux-cifs@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---
fs/netfs/rolling_buffer.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/fs/netfs/rolling_buffer.c b/fs/netfs/rolling_buffer.c
index 75d97af14b4a..207b6a326651 100644
--- a/fs/netfs/rolling_buffer.c
+++ b/fs/netfs/rolling_buffer.c
@@ -146,10 +146,6 @@ ssize_t rolling_buffer_load_from_ra(struct rolling_buffer *roll,
/* Store the counter after setting the slot. */
smp_store_release(&roll->next_head_slot, to);
-
- for (; ix < folioq_nr_slots(fq); ix++)
- folioq_clear(fq, ix);
-
return size;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/4] netfs: Fix netfs_unbuffered_read() to return ssize_t rather than int
2025-03-14 16:41 [PATCH 0/4] netfs: Miscellaneous fixes David Howells
` (2 preceding siblings ...)
2025-03-14 16:41 ` [PATCH 3/4] netfs: Fix rolling_buffer_load_from_ra() to not clear mark bits David Howells
@ 2025-03-14 16:41 ` David Howells
2025-03-14 20:44 ` [PATCH 0/4] netfs: Miscellaneous fixes Paulo Alcantara
2025-03-19 9:04 ` Christian Brauner
5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2025-03-14 16:41 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Max Kellermann, Jeff Layton, netfs, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel,
linux-kernel, Viacheslav Dubeyko, Alex Markuze, Ilya Dryomov
Fix netfs_unbuffered_read() to return an ssize_t rather than an int as
netfs_wait_for_read() returns ssize_t and this gets implicitly truncated.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: Viacheslav Dubeyko <slava@dubeyko.com>
cc: Alex Markuze <amarkuze@redhat.com>
cc: Ilya Dryomov <idryomov@gmail.com>
cc: ceph-devel@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---
fs/netfs/direct_read.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/netfs/direct_read.c b/fs/netfs/direct_read.c
index 0bf3c2f5a710..5e3f0aeb51f3 100644
--- a/fs/netfs/direct_read.c
+++ b/fs/netfs/direct_read.c
@@ -125,9 +125,9 @@ static int netfs_dispatch_unbuffered_reads(struct netfs_io_request *rreq)
* Perform a read to an application buffer, bypassing the pagecache and the
* local disk cache.
*/
-static int netfs_unbuffered_read(struct netfs_io_request *rreq, bool sync)
+static ssize_t netfs_unbuffered_read(struct netfs_io_request *rreq, bool sync)
{
- int ret;
+ ssize_t ret;
_enter("R=%x %llx-%llx",
rreq->debug_id, rreq->start, rreq->start + rreq->len - 1);
@@ -155,7 +155,7 @@ static int netfs_unbuffered_read(struct netfs_io_request *rreq, bool sync)
else
ret = -EIOCBQUEUED;
out:
- _leave(" = %d", ret);
+ _leave(" = %zd", ret);
return ret;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/4] netfs: Miscellaneous fixes
2025-03-14 16:41 [PATCH 0/4] netfs: Miscellaneous fixes David Howells
` (3 preceding siblings ...)
2025-03-14 16:41 ` [PATCH 4/4] netfs: Fix netfs_unbuffered_read() to return ssize_t rather than int David Howells
@ 2025-03-14 20:44 ` Paulo Alcantara
2025-03-19 9:04 ` Christian Brauner
5 siblings, 0 replies; 9+ messages in thread
From: Paulo Alcantara @ 2025-03-14 20:44 UTC (permalink / raw)
To: David Howells, Christian Brauner
Cc: David Howells, Max Kellermann, Jeff Layton, netfs, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel,
linux-kernel
David Howells <dhowells@redhat.com> writes:
> Hi Christian,
>
> Here are some miscellaneous fixes and changes for netfslib, if you could
> pull them:
>
> (1) Fix the collection of results during a pause in transmission.
>
> (2) Call ->invalidate_cache() only if provided.
>
> (3) Fix the rolling buffer to not hammer atomic bit clears when loading
> from readahead.
>
> (4) Fix netfs_unbuffered_read() to return ssize_t.
>
> The patches can also be found here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=netfs-fixes
>
> Thanks,
> David
>
> David Howells (3):
> netfs: Fix collection of results during pause when collection
> offloaded
> netfs: Fix rolling_buffer_load_from_ra() to not clear mark bits
> netfs: Fix netfs_unbuffered_read() to return ssize_t rather than int
>
> Max Kellermann (1):
> netfs: Call `invalidate_cache` only if implemented
>
> fs/netfs/direct_read.c | 6 +++---
> fs/netfs/read_collect.c | 18 ++++++++++--------
> fs/netfs/rolling_buffer.c | 4 ----
> fs/netfs/write_collect.c | 3 ++-
> 4 files changed, 15 insertions(+), 16 deletions(-)
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/4] netfs: Miscellaneous fixes
2025-03-14 16:41 [PATCH 0/4] netfs: Miscellaneous fixes David Howells
` (4 preceding siblings ...)
2025-03-14 20:44 ` [PATCH 0/4] netfs: Miscellaneous fixes Paulo Alcantara
@ 2025-03-19 9:04 ` Christian Brauner
5 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2025-03-19 9:04 UTC (permalink / raw)
To: David Howells
Cc: Christian Brauner, Max Kellermann, Jeff Layton, netfs, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel,
linux-kernel
On Fri, 14 Mar 2025 16:41:55 +0000, David Howells wrote:
> Here are some miscellaneous fixes and changes for netfslib, if you could
> pull them:
>
> (1) Fix the collection of results during a pause in transmission.
>
> (2) Call ->invalidate_cache() only if provided.
>
> [...]
Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes
[1/4] netfs: Fix collection of results during pause when collection offloaded
https://git.kernel.org/vfs/vfs/c/f298e3765528
[2/4] netfs: Call `invalidate_cache` only if implemented
https://git.kernel.org/vfs/vfs/c/344b7ef248f4
[3/4] netfs: Fix rolling_buffer_load_from_ra() to not clear mark bits
https://git.kernel.org/vfs/vfs/c/15e9aaf9fc49
[4/4] netfs: Fix netfs_unbuffered_read() to return ssize_t rather than int
https://git.kernel.org/vfs/vfs/c/07c574eb53d4
^ permalink raw reply [flat|nested] 9+ messages in thread