netfs.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] netfs: Miscellaneous fixes
@ 2025-03-14 16:41 David Howells
  2025-03-14 16:41 ` [PATCH 1/4] netfs: Fix collection of results during pause when collection offloaded David Howells
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ 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

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(-)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread

end of thread, other threads:[~2025-03-19  9:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [PATCH 3/4] netfs: Fix rolling_buffer_load_from_ra() to not clear mark bits David Howells
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 ` [PATCH 0/4] netfs: Miscellaneous fixes Paulo Alcantara
2025-03-19  9:04 ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).