Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230608214137.856006-1-dwysocha@redhat.com>
In-Reply-To: <20230608214137.856006-1-dwysocha@redhat.com>
From:   David Wysochanski <dwysocha@redhat.com>
Date:   Mon, 24 Jul 2023 10:20:59 -0400
Message-ID: <CALF+zOk+r18aDDJ19Ngt2N0G3CPeuXD8vVh7cqZ5s+Up9Kw4Uw@mail.gmail.com>
Subject: Re: [Linux-cachefs] [PATCH] netfs: Only call folio_start_fscache()
 one time for each folio
To:     David Howells <dhowells@redhat.com>
Cc:     linux-nfs@vger.kernel.org, linux-cachefs@redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Jun 8, 2023 at 5:41=E2=80=AFPM Dave Wysochanski <dwysocha@redhat.co=
m> wrote:
>
> If a network filesystem using netfs implements a clamp_length()
> function, it can set subrequest lengths smaller than a page size.
> When we loop through the folios in netfs_rreq_unlock_folios() to
> set any folios to be written back, we need to make sure we only
> call folio_start_fscache() once for each folio.  Otherwise,
> this simple testcase:
>   mount -o fsc,rsize=3D1024,wsize=3D1024 127.0.0.1:/export /mnt/nfs
>   dd if=3D/dev/zero of=3D/mnt/nfs/file.bin bs=3D4096 count=3D1
>   1+0 records in
>   1+0 records out
>   4096 bytes (4.1 kB, 4.0 KiB) copied, 0.0126359 s, 324 kB/s
>   cat /mnt/nfs/file.bin > /dev/null
>
> will trigger an oops similar to the following:
> ...
>  page dumped because: VM_BUG_ON_FOLIO(folio_test_private_2(folio))
>  ------------[ cut here ]------------
>  kernel BUG at include/linux/netfs.h:44!
> ...
>  CPU: 5 PID: 134 Comm: kworker/u16:5 Kdump: loaded Not tainted 6.4.0-rc5
> ...
>  RIP: 0010:netfs_rreq_unlock_folios+0x68e/0x730 [netfs]
> ...
>  Call Trace:
>   <TASK>
>   netfs_rreq_assess+0x497/0x660 [netfs]
>   netfs_subreq_terminated+0x32b/0x610 [netfs]
>   nfs_netfs_read_completion+0x14e/0x1a0 [nfs]
>   nfs_read_completion+0x2f9/0x330 [nfs]
>   rpc_free_task+0x72/0xa0 [sunrpc]
>   rpc_async_release+0x46/0x70 [sunrpc]
>   process_one_work+0x3bd/0x710
>   worker_thread+0x89/0x610
>   kthread+0x181/0x1c0
>   ret_from_fork+0x29/0x50
>
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> ---
>  fs/netfs/buffered_read.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
> index 3404707ddbe7..0dafd970c1b6 100644
> --- a/fs/netfs/buffered_read.c
> +++ b/fs/netfs/buffered_read.c
> @@ -21,6 +21,7 @@ void netfs_rreq_unlock_folios(struct netfs_io_request *=
rreq)
>         pgoff_t last_page =3D ((rreq->start + rreq->len) / PAGE_SIZE) - 1=
;
>         size_t account =3D 0;
>         bool subreq_failed =3D false;
> +       bool folio_started;
>
>         XA_STATE(xas, &rreq->mapping->i_pages, start_page);
>
> @@ -53,6 +54,7 @@ void netfs_rreq_unlock_folios(struct netfs_io_request *=
rreq)
>
>                 pg_end =3D folio_pos(folio) + folio_size(folio) - 1;
>
> +               folio_started =3D false;
>                 for (;;) {
>                         loff_t sreq_end;
>
> @@ -60,8 +62,10 @@ void netfs_rreq_unlock_folios(struct netfs_io_request =
*rreq)
>                                 pg_failed =3D true;
>                                 break;
>                         }
> -                       if (test_bit(NETFS_SREQ_COPY_TO_CACHE, &subreq->f=
lags))
> +                       if (!folio_started && test_bit(NETFS_SREQ_COPY_TO=
_CACHE, &subreq->flags)) {
>                                 folio_start_fscache(folio);
> +                               folio_started =3D true;
> +                       }
>                         pg_failed |=3D subreq_failed;
>                         sreq_end =3D subreq->start + subreq->len - 1;
>                         if (pg_end < sreq_end)
> --
> 2.31.1
>
> --
> Linux-cachefs mailing list
> Linux-cachefs@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-cachefs
>

David,

Just wanted to ping a friendly reminder on this patch as I didn't see
any response or in any tree that I could find.

Also, there is a Red Hat bugzilla for it, so patch should have had:
Link: https://bugzilla.redhat.com/show_bug.cgi?id=3D2210612