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

[Christoph Hellwig <hch@lst.de>]

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.
> 
> Additionally, refactor nfs_read_sync_pgio_error() to utilize this new
> helper.
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>  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.