Unverified Commit b5782e2d authored by David Howells's avatar David Howells Committed by Christian Brauner
Browse files

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: e2d46f2e ("netfs: Change the read result collector to only use one work item")
Fixes: 288ace2f ("netfs: New writeback implementation")
Link: https://sashiko.dev/#/patchset/20260326104544.509518-1-dhowells%40redhat.com


Signed-off-by: default avatarDavid 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: default avatarChristian Brauner <brauner@kernel.org>
parent cce18c26

fs/netfs/buffered_read.c

+2 −1
Original line number Diff line number Diff line
@@ -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;

fs/netfs/misc.c

+1 −0
Original line number Diff line number Diff line
@@ -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;

fs/netfs/read_collect.c

+4 −2
Original line number Diff line number Diff line
@@ -205,8 +205,10 @@ static void netfs_collect_read_results(struct netfs_io_request *rreq) 
	 * 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,

	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;

fs/netfs/write_collect.c

+4 −2
Original line number Diff line number Diff line
@@ -228,8 +228,10 @@ static void netfs_collect_write_results(struct netfs_io_request *wreq) 
		if (!smp_load_acquire(&stream->active))

			continue;



		front = list_first_entry_or_null(&stream->subrequests,

		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",

fs/netfs/write_issue.c

+2 −1
Original line number Diff line number Diff line
@@ -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;