netfs: Fix missing barriers when accessing stream->subrequests locklessly

The list of subrequests attached to stream->subrequests is accessed without
locks by netfs_collect_read_results() and netfs_collect_write_results(),
and then they access subreq->flags without taking a barrier after getting
the subreq pointer from the list.  Relatedly, the functions that build the
list don't use any sort of write barrier when constructing the list to make
sure that the NETFS_SREQ_IN_PROGRESS flag is perceived to be set first if
no lock is taken.

Fix this by:

 (1) Add a new list_add_tail_release() function that uses a release barrier
     to set the pointer to the new member of the list.

 (2) Add a new list_first_entry_or_null_acquire() function that uses an
     acquire barrier to read the pointer to the first member in a list (or
     return NULL).

 (3) Use list_add_tail_release() when adding a subreq to ->subrequests.

 (4) Use list_first_entry_or_null_acquire() when initially accessing the
     front of the list (when an item is removed, the pointer to the new
     front iterm is obtained under the same lock).

Fixes: e2d46f2ec332 ("netfs: Change the read result collector to only use one work item")
Fixes: 288ace2f57c9 ("netfs: New writeback implementation")
Link: https://sashiko.dev/#/patchset/20260326104544.509518-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://patch.msgid.link/20260512123404.719402-4-dhowells@redhat.com
cc: Paulo Alcantara <pc@manguebit.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index a27ed501b6d436aec2432dfa8b7aebca3d98660e..15d73026ff64384743d31fc28fd5c10e0c87bce2 100644 (file)
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -168,7 +168,8 @@ void netfs_queue_read(struct netfs_io_request *rreq,
         * remove entries off of the front.
         */
        spin_lock(&rreq->lock);
-       list_add_tail(&subreq->rreq_link, &stream->subrequests);
+       /* Write IN_PROGRESS before pointer to new subreq */
+       list_add_tail_release(&subreq->rreq_link, &stream->subrequests);
        if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
                if (!stream->active) {
                        stream->collected_to = subreq->start;

diff --git a/fs/netfs/misc.c b/fs/netfs/misc.c
index 6df89c92b10b0656aeee12ce5d6a431564dde53f..21357907b7eee9a1cc268be4abeed415efd2859e 100644 (file)
--- a/fs/netfs/misc.c
+++ b/fs/netfs/misc.c
@@ -356,6 +356,7 @@ void netfs_wait_for_in_progress_stream(struct netfs_io_request *rreq,
        DEFINE_WAIT(myself);
 
        list_for_each_entry(subreq, &stream->subrequests, rreq_link) {
+               smp_rmb(); /* Read ->next before IN_PROGRESS. */
                if (!netfs_check_subreq_in_progress(subreq))
                        continue;
 

diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index d2d902f466271d99f08bf85369d27ce101300a81..3c9b847885c2a8a46d99033fb671f8e3c3f5dfa1 100644 (file)
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -205,8 +205,10 @@ reassess:
         * in progress.  The issuer thread may be adding stuff to the tail
         * whilst we're doing this.
         */
-       front = list_first_entry_or_null(&stream->subrequests,
-                                        struct netfs_io_subrequest, rreq_link);
+       front = list_first_entry_or_null_acquire(&stream->subrequests,
+                                                struct netfs_io_subrequest, rreq_link);
+       /* Read first subreq pointer before IN_PROGRESS flag. */
+
        while (front) {
                size_t transferred;
 

diff --git a/fs/netfs/write_collect.c b/fs/netfs/write_collect.c
index b194447f4b111711d8ac79d0d049ba11d0cec3d0..7fbf50907a7fc615dbfd1027c8caaf162300b710 100644 (file)
--- a/fs/netfs/write_collect.c
+++ b/fs/netfs/write_collect.c
@@ -228,8 +228,10 @@ reassess_streams:
                if (!smp_load_acquire(&stream->active))
                        continue;
 
-               front = list_first_entry_or_null(&stream->subrequests,
-                                                struct netfs_io_subrequest, rreq_link);
+               front = list_first_entry_or_null_acquire(&stream->subrequests,
+                                                        struct netfs_io_subrequest, rreq_link);
+               /* Read first subreq pointer before IN_PROGRESS flag. */
+
                while (front) {
                        trace_netfs_collect_sreq(wreq, front);
                        //_debug("sreq [%x] %llx %zx/%zx",

diff --git a/fs/netfs/write_issue.c b/fs/netfs/write_issue.c
index 2db688f94125195d567e24a8aa52a01d1a66ecb8..b0e9690bb90ce6cb30d91bbc77417fdd83d6b98c 100644 (file)
--- a/fs/netfs/write_issue.c
+++ b/fs/netfs/write_issue.c
@@ -204,7 +204,8 @@ void netfs_prepare_write(struct netfs_io_request *wreq,
         * remove entries off of the front.
         */
        spin_lock(&wreq->lock);
-       list_add_tail(&subreq->rreq_link, &stream->subrequests);
+       /* Write IN_PROGRESS before pointer to new subreq */
+       list_add_tail_release(&subreq->rreq_link, &stream->subrequests);
        if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
                if (!stream->active) {
                        stream->collected_to = subreq->start;

diff --git a/include/linux/list.h b/include/linux/list.h
index 00ea8e5fb88b0d86b153437e7f4250ff47e4460f..09d979976b3b865a3fe7a0182530a1820be5b0fb 100644 (file)
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -191,6 +191,29 @@ static inline void list_add_tail(struct list_head *new, struct list_head *head)
        __list_add(new, head->prev, head);
 }
 
+/**
+ * list_add_tail_release - add a new entry with release barrier
+ * @new: new entry to be added
+ * @head: list head to add it before
+ *
+ * Insert a new entry before the specified head, using a release barrier to set
+ * the ->next pointer that points to it.  This is useful for implementing
+ * queues, in particular one that the elements will be walked through forwards
+ * locklessly.
+ */
+static inline void list_add_tail_release(struct list_head *new,
+                                        struct list_head *head)
+{
+       struct list_head *prev = head->prev;
+
+       if (__list_add_valid(new, prev, head)) {
+               new->next = head;
+               new->prev = prev;
+               head->prev = new;
+               smp_store_release(&prev->next, new);
+       }
+}
+
 /*
  * Delete a list entry by making the prev/next entries
  * point to each other.
@@ -644,6 +667,20 @@ static inline void list_splice_tail_init(struct list_head *list,
        pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
 })
 
+/**
+ * list_first_entry_or_null_acquire - get the first element from a list with barrier
+ * @ptr:       the list head to take the element from.
+ * @type:      the type of the struct this is embedded in.
+ * @member:    the name of the list_head within the struct.
+ *
+ * Note that if the list is empty, it returns NULL.
+ */
+#define list_first_entry_or_null_acquire(ptr, type, member) ({ \
+       struct list_head *head__ = (ptr); \
+       struct list_head *pos__ = smp_load_acquire(&head__->next); \
+       pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
+})
+
 /**
  * list_last_entry_or_null - get the last element from a list
  * @ptr:       the list head to take the element from.