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: a0b4c7a491 ("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>
This commit is contained in:
parent
f621324dfb
commit
0e764b9d46
|
|
@ -171,9 +171,8 @@ static void netfs_queue_read(struct netfs_io_request *rreq,
|
||||||
spin_lock(&rreq->lock);
|
spin_lock(&rreq->lock);
|
||||||
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
||||||
if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
|
if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
|
||||||
stream->front = subreq;
|
|
||||||
if (!stream->active) {
|
if (!stream->active) {
|
||||||
stream->collected_to = stream->front->start;
|
stream->collected_to = subreq->start;
|
||||||
/* Store list pointers before active flag */
|
/* Store list pointers before active flag */
|
||||||
smp_store_release(&stream->active, true);
|
smp_store_release(&stream->active, true);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -71,9 +71,8 @@ static int netfs_dispatch_unbuffered_reads(struct netfs_io_request *rreq)
|
||||||
spin_lock(&rreq->lock);
|
spin_lock(&rreq->lock);
|
||||||
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
||||||
if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
|
if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
|
||||||
stream->front = subreq;
|
|
||||||
if (!stream->active) {
|
if (!stream->active) {
|
||||||
stream->collected_to = stream->front->start;
|
stream->collected_to = subreq->start;
|
||||||
/* Store list pointers before active flag */
|
/* Store list pointers before active flag */
|
||||||
smp_store_release(&stream->active, true);
|
smp_store_release(&stream->active, true);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -111,7 +111,6 @@ static int netfs_unbuffered_write(struct netfs_io_request *wreq)
|
||||||
netfs_prepare_write(wreq, stream, wreq->start + wreq->transferred);
|
netfs_prepare_write(wreq, stream, wreq->start + wreq->transferred);
|
||||||
subreq = stream->construct;
|
subreq = stream->construct;
|
||||||
stream->construct = NULL;
|
stream->construct = NULL;
|
||||||
stream->front = NULL;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Check if (re-)preparation failed. */
|
/* Check if (re-)preparation failed. */
|
||||||
|
|
|
||||||
|
|
@ -205,7 +205,8 @@ reassess:
|
||||||
* in progress. The issuer thread may be adding stuff to the tail
|
* in progress. The issuer thread may be adding stuff to the tail
|
||||||
* whilst we're doing this.
|
* 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) {
|
while (front) {
|
||||||
size_t transferred;
|
size_t transferred;
|
||||||
|
|
||||||
|
|
@ -301,7 +302,6 @@ reassess:
|
||||||
list_del_init(&front->rreq_link);
|
list_del_init(&front->rreq_link);
|
||||||
front = list_first_entry_or_null(&stream->subrequests,
|
front = list_first_entry_or_null(&stream->subrequests,
|
||||||
struct netfs_io_subrequest, rreq_link);
|
struct netfs_io_subrequest, rreq_link);
|
||||||
stream->front = front;
|
|
||||||
spin_unlock(&rreq->lock);
|
spin_unlock(&rreq->lock);
|
||||||
netfs_put_subrequest(remove,
|
netfs_put_subrequest(remove,
|
||||||
notes & ABANDON_SREQ ?
|
notes & ABANDON_SREQ ?
|
||||||
|
|
|
||||||
|
|
@ -107,7 +107,6 @@ static int netfs_single_dispatch_read(struct netfs_io_request *rreq)
|
||||||
spin_lock(&rreq->lock);
|
spin_lock(&rreq->lock);
|
||||||
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
||||||
trace_netfs_sreq(subreq, netfs_sreq_trace_added);
|
trace_netfs_sreq(subreq, netfs_sreq_trace_added);
|
||||||
stream->front = subreq;
|
|
||||||
/* Store list pointers before active flag */
|
/* Store list pointers before active flag */
|
||||||
smp_store_release(&stream->active, true);
|
smp_store_release(&stream->active, true);
|
||||||
spin_unlock(&rreq->lock);
|
spin_unlock(&rreq->lock);
|
||||||
|
|
|
||||||
|
|
@ -228,7 +228,8 @@ reassess_streams:
|
||||||
if (!smp_load_acquire(&stream->active))
|
if (!smp_load_acquire(&stream->active))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
front = stream->front;
|
front = list_first_entry_or_null(&stream->subrequests,
|
||||||
|
struct netfs_io_subrequest, rreq_link);
|
||||||
while (front) {
|
while (front) {
|
||||||
trace_netfs_collect_sreq(wreq, front);
|
trace_netfs_collect_sreq(wreq, front);
|
||||||
//_debug("sreq [%x] %llx %zx/%zx",
|
//_debug("sreq [%x] %llx %zx/%zx",
|
||||||
|
|
@ -279,7 +280,6 @@ reassess_streams:
|
||||||
list_del_init(&front->rreq_link);
|
list_del_init(&front->rreq_link);
|
||||||
front = list_first_entry_or_null(&stream->subrequests,
|
front = list_first_entry_or_null(&stream->subrequests,
|
||||||
struct netfs_io_subrequest, rreq_link);
|
struct netfs_io_subrequest, rreq_link);
|
||||||
stream->front = front;
|
|
||||||
spin_unlock(&wreq->lock);
|
spin_unlock(&wreq->lock);
|
||||||
netfs_put_subrequest(remove,
|
netfs_put_subrequest(remove,
|
||||||
notes & SAW_FAILURE ?
|
notes & SAW_FAILURE ?
|
||||||
|
|
|
||||||
|
|
@ -206,9 +206,8 @@ void netfs_prepare_write(struct netfs_io_request *wreq,
|
||||||
spin_lock(&wreq->lock);
|
spin_lock(&wreq->lock);
|
||||||
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
list_add_tail(&subreq->rreq_link, &stream->subrequests);
|
||||||
if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
|
if (list_is_first(&subreq->rreq_link, &stream->subrequests)) {
|
||||||
stream->front = subreq;
|
|
||||||
if (!stream->active) {
|
if (!stream->active) {
|
||||||
stream->collected_to = stream->front->start;
|
stream->collected_to = subreq->start;
|
||||||
/* Write list pointers before active flag */
|
/* Write list pointers before active flag */
|
||||||
smp_store_release(&stream->active, true);
|
smp_store_release(&stream->active, true);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -140,7 +140,6 @@ struct netfs_io_stream {
|
||||||
void (*issue_write)(struct netfs_io_subrequest *subreq);
|
void (*issue_write)(struct netfs_io_subrequest *subreq);
|
||||||
/* Collection tracking */
|
/* Collection tracking */
|
||||||
struct list_head subrequests; /* Contributory I/O operations */
|
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 */
|
unsigned long long collected_to; /* Position we've collected results to */
|
||||||
size_t transferred; /* The amount transferred from this stream */
|
size_t transferred; /* The amount transferred from this stream */
|
||||||
unsigned short error; /* Aggregate error for the stream */
|
unsigned short error; /* Aggregate error for the stream */
|
||||||
|
|
|
||||||
|
|
@ -740,19 +740,19 @@ TRACE_EVENT(netfs_collect_stream,
|
||||||
__field(unsigned int, wreq)
|
__field(unsigned int, wreq)
|
||||||
__field(unsigned char, stream)
|
__field(unsigned char, stream)
|
||||||
__field(unsigned long long, collected_to)
|
__field(unsigned long long, collected_to)
|
||||||
__field(unsigned long long, front)
|
__field(unsigned long long, issued_to)
|
||||||
),
|
),
|
||||||
|
|
||||||
TP_fast_assign(
|
TP_fast_assign(
|
||||||
__entry->wreq = wreq->debug_id;
|
__entry->wreq = wreq->debug_id;
|
||||||
__entry->stream = stream->stream_nr;
|
__entry->stream = stream->stream_nr;
|
||||||
__entry->collected_to = stream->collected_to;
|
__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->wreq, __entry->stream,
|
||||||
__entry->collected_to, __entry->front)
|
__entry->collected_to, __entry->issued_to)
|
||||||
);
|
);
|
||||||
|
|
||||||
TRACE_EVENT(netfs_folioq,
|
TRACE_EVENT(netfs_folioq,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue