netfs: Fix the handling of stream->front by removing it

The netfs_io_stream::front member is meant to point to the subrequest
currently being collected on a stream, but it isn't actually used this way
by direct write (which mostly ignores it).  However, there's a tracepoint
which looks at it.  Further, stream->front is actually redundant with
stream->subrequests.next.

Fix the potential problem in the direct code by just removing the member
and using stream->subrequests.next instead, thereby also simplifying the
code.

Fixes: a0b4c7a49137 ("netfs: Fix unbuffered/DIO writes to dispatch subrequests in strict sequence")
Reported-by: Paulo Alcantara <pc@manguebit.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://patch.msgid.link/4158599.1774426817@warthog.procyon.org.uk
Reviewed-by: Paulo Alcantara (Red Hat) <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 88a0d801525f9b0466807c4c7c79e5ba633dd94a..a8c0d86118c5851f6f1339cfc1a789db75da647b 100644 (file)
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -171,9 +171,8 @@ static void netfs_queue_read(struct netfs_io_request *rreq,
        spin_lock(&rreq->lock);
        list_add_tail(&subreq->rreq_link, &stream->subrequests);
        if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
-               stream->front = subreq;
                if (!stream->active) {
-                       stream->collected_to = stream->front->start;
+                       stream->collected_to = subreq->start;
                        /* Store list pointers before active flag */
                        smp_store_release(&stream->active, true);
                }

diff --git a/fs/netfs/direct_read.c b/fs/netfs/direct_read.c
index a498ee8d66745f28cc070b9e0d1ca9105316bd99..f72e6da88cca7c45eb451a0dc422cae79de2be1a 100644 (file)
--- a/fs/netfs/direct_read.c
+++ b/fs/netfs/direct_read.c
@@ -71,9 +71,8 @@ static int netfs_dispatch_unbuffered_reads(struct netfs_io_request *rreq)
                spin_lock(&rreq->lock);
                list_add_tail(&subreq->rreq_link, &stream->subrequests);
                if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
-                       stream->front = subreq;
                        if (!stream->active) {
-                               stream->collected_to = stream->front->start;
+                               stream->collected_to = subreq->start;
                                /* Store list pointers before active flag */
                                smp_store_release(&stream->active, true);
                        }

diff --git a/fs/netfs/direct_write.c b/fs/netfs/direct_write.c
index 4d9760e36c1194b615540d7feea3a6d931d51afc..f9ab69de3e298aef1f0fae1014155d8bef27060c 100644 (file)
--- a/fs/netfs/direct_write.c
+++ b/fs/netfs/direct_write.c
@@ -111,7 +111,6 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq)
                        netfs_prepare_write(wreq, stream, wreq->start + wreq->transferred);
                        subreq = stream->construct;
                        stream->construct = NULL;
-                       stream->front = NULL;
                }
 
                /* Check if (re-)preparation failed. */

diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index 137f0e28a44c522aa86579cdbc688b8b2130f6fb..e5f6665b3341ee27b1bbc8dbf09990d9eb4a453d 100644 (file)
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -205,7 +205,8 @@ reassess:
         * in progress.  The issuer thread may be adding stuff to the tail
         * whilst we're doing this.
         */
-       front = READ_ONCE(stream->front);
+       front = list_first_entry_or_null(&stream->subrequests,
+                                        struct netfs_io_subrequest, rreq_link);
        while (front) {
                size_t transferred;
 
@@ -301,7 +302,6 @@ reassess:
                list_del_init(&front->rreq_link);
                front = list_first_entry_or_null(&stream->subrequests,
                                                 struct netfs_io_subrequest, rreq_link);
-               stream->front = front;
                spin_unlock(&rreq->lock);
                netfs_put_subrequest(remove,
                                     notes & ABANDON_SREQ ?

diff --git a/fs/netfs/read_single.c b/fs/netfs/read_single.c
index 8e6264f62a8f381fefc14905d8c31cb851414841..d0e23bc42445ffde88deed785e14cf9eed8f1590 100644 (file)
--- a/fs/netfs/read_single.c
+++ b/fs/netfs/read_single.c
@@ -107,7 +107,6 @@ static int netfs_single_dispatch_read(struct netfs_io_request *rreq)
        spin_lock(&rreq->lock);
        list_add_tail(&subreq->rreq_link, &stream->subrequests);
        trace_netfs_sreq(subreq, netfs_sreq_trace_added);
-       stream->front = subreq;
        /* Store list pointers before active flag */
        smp_store_release(&stream->active, true);
        spin_unlock(&rreq->lock);

diff --git a/fs/netfs/write_collect.c b/fs/netfs/write_collect.c
index 83eb3dc1adf8a15524d83c32db34c22edce53655..b194447f4b111711d8ac79d0d049ba11d0cec3d0 100644 (file)
--- a/fs/netfs/write_collect.c
+++ b/fs/netfs/write_collect.c
@@ -228,7 +228,8 @@ reassess_streams:
                if (!smp_load_acquire(&stream->active))
                        continue;
 
-               front = stream->front;
+               front = list_first_entry_or_null(&stream->subrequests,
+                                                struct netfs_io_subrequest, rreq_link);
                while (front) {
                        trace_netfs_collect_sreq(wreq, front);
                        //_debug("sreq [%x] %llx %zx/%zx",
@@ -279,7 +280,6 @@ reassess_streams:
                        list_del_init(&front->rreq_link);
                        front = list_first_entry_or_null(&stream->subrequests,
                                                         struct netfs_io_subrequest, rreq_link);
-                       stream->front = front;
                        spin_unlock(&wreq->lock);
                        netfs_put_subrequest(remove,
                                             notes & SAW_FAILURE ?

diff --git a/fs/netfs/write_issue.c b/fs/netfs/write_issue.c
index 437268f6564095d833629aaa8b52dc0158c94461..2db688f94125195d567e24a8aa52a01d1a66ecb8 100644 (file)
--- a/fs/netfs/write_issue.c
+++ b/fs/netfs/write_issue.c
@@ -206,9 +206,8 @@ void netfs_prepare_write(struct netfs_io_request *wreq,
        spin_lock(&wreq->lock);
        list_add_tail(&subreq->rreq_link, &stream->subrequests);
        if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
-               stream->front = subreq;
                if (!stream->active) {
-                       stream->collected_to = stream->front->start;
+                       stream->collected_to = subreq->start;
                        /* Write list pointers before active flag */
                        smp_store_release(&stream->active, true);
                }

diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 72ee7d210a7445bc67934f5dfd9a9cc33887f2f8..ba17ac5bf356ae8fb5da97e7082d8769d981cd67 100644 (file)
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -140,7 +140,6 @@ struct netfs_io_stream {
        void (*issue_write)(struct netfs_io_subrequest *subreq);
        /* Collection tracking */
        struct list_head        subrequests;    /* Contributory I/O operations */
-       struct netfs_io_subrequest *front;      /* Op being collected */
        unsigned long long      collected_to;   /* Position we've collected results to */
        size_t                  transferred;    /* The amount transferred from this stream */
        unsigned short          error;          /* Aggregate error for the stream */

diff --git a/include/trace/events/netfs.h b/include/trace/events/netfs.h
index 2d366be46a1c30feba5ac2368492d7671687a229..cbe28211106c52abe8fc091b71670033bb94c3f8 100644 (file)
--- a/include/trace/events/netfs.h
+++ b/include/trace/events/netfs.h
@@ -740,19 +740,19 @@ TRACE_EVENT(netfs_collect_stream,
                    __field(unsigned int,       wreq)
                    __field(unsigned char,      stream)
                    __field(unsigned long long, collected_to)
-                   __field(unsigned long long, front)
+                   __field(unsigned long long, issued_to)
                             ),
 
            TP_fast_assign(
                    __entry->wreq       = wreq->debug_id;
                    __entry->stream     = stream->stream_nr;
                    __entry->collected_to = stream->collected_to;
-                   __entry->front      = stream->front ? stream->front->start : UINT_MAX;
+                   __entry->issued_to  = atomic64_read(&wreq->issued_to);
                           ),
 
-           TP_printk("R=%08x[%x:] cto=%llx frn=%llx",
+           TP_printk("R=%08x[%x:] cto=%llx ito=%llx",
                      __entry->wreq, __entry->stream,
-                     __entry->collected_to, __entry->front)
+                     __entry->collected_to, __entry->issued_to)
            );
 
 TRACE_EVENT(netfs_folioq,