Re: [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper

On Mon, Jul 20, 2026 at 2:45 PM Christoph Hellwig <[email protected]> wrote:
>
> On Wed, Jul 15, 2026 at 02:35:38PM +0000, Pranjal Shrivastava wrote:
> > Introduce a centralized helper, nfs_release_request_list, to handle
> > the bulk release of nfs_page requests from a list.
> >
> > This serves as a preparatory step for two upcoming improvements:
> >
> >    1. Pin-Aware Cleanup: As we migrate to iov_iter_extract_* API,
> >       requests will hold pins (GUP) instead of standard references. The
> >       helper ensures that the correct unpinning logic gets applied
> >       consistently across all requests in a list.
> >
> >    2. Folio Support: In subsequent patches where nfs_page structures
> >       will cover multi-page folios, this helper provides a clean
> >       infrastructure to unlock these larger units of I/O in bulk during
> >       completion, similat to the pattern in bio_release_pages.

nit: similar -> similar


> >
> > Additionally, refactor nfs_read_sync_pgio_error() to utilize this new
> > helper.
> >
> > Signed-off-by: Pranjal Shrivastava <[email protected]>
> > ---
> >  fs/nfs/direct.c          |  8 +-------
> >  fs/nfs/pagelist.c        | 18 ++++++++++++++++++
> >  include/linux/nfs_page.h |  4 ++--
> >  3 files changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> > index 19792a38c924..96995736fac2 100644
> > --- a/fs/nfs/direct.c
> > +++ b/fs/nfs/direct.c
> > @@ -314,13 +314,7 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
> >
> >  static void nfs_read_sync_pgio_error(struct list_head *head, int error)
> >  {
> > -     struct nfs_page *req;
> > -
> > -     while (!list_empty(head)) {
> > -             req = nfs_list_entry(head->next);
> > -             nfs_list_remove_request(req);
> > -             nfs_release_request(req);
> > -     }
> > +     nfs_release_request_list(head);
> >  }
> >
> >  static void nfs_direct_pgio_init(struct nfs_pgio_header *hdr)
> > diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> > index 7d51e10fe97a..569bac4faff7 100644
> > --- a/fs/nfs/pagelist.c
> > +++ b/fs/nfs/pagelist.c
> > @@ -622,6 +622,24 @@ void nfs_release_request(struct nfs_page *req)
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_release_request);
> >
> > +/*
> > + * nfs_release_request_list - Release a list of NFS read/write requests
> > + * @head: list of requests to release
> > + *
> > + * Removes each request from the list and drops it's refcount.
> > + */
> > +void nfs_release_request_list(struct list_head *head)
> > +{
> > +     struct nfs_page *req;
> > +
> > +     while (!list_empty(head)) {
> > +             req = nfs_list_entry(head->next);
>
> req could/should be local here.  Or you switch to list_first_or_null,
> which some folks prefer.
>
> > +extern void nfs_release_request(struct nfs_page *req);
> > +extern void nfs_release_request_list(struct list_head *head);
>
> Please drop the superflous externs for new code.
>

Small nit + fix christoph's comment,
else looks ok

Reviewed-by: Shivaji Kant <[email protected]>