[Devel] [PATCH vz10] nfsd: release async COPY nf_src/nf_dst when stopping a copy

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

On 6/12/26 00:13, Konstantin Khorenko wrote:
> An asynchronous server-side NFSv4 COPY (struct nfsd4_copy) pins its
> source and destination files in nf_src/nf_dst. These references are
> dropped only by cleanup_async_copy(), which runs from the offload
> reaper (for OFFLOAD_DONE copies) and from the copy setup error path.
> 
> nfsd4_stop_copy() - used to abort a copy on OFFLOAD_CANCEL and during
> client teardown via nfsd4_shutdown_copy() - stops the copy kthread and
> drops the copy refcount, but never releases nf_src/nf_dst. The aborted
> copy has already been taken off clp->async_copies (cp_clp set to NULL),
> so the reaper can no longer reach it either. As a result every
> cancelled or torn-down async COPY leaks its two nfsd_file references.
> 
> When nfsd is later shut down ("nfsdctl threads 0" -> nfsd_destroy_serv
> -> nfsd_file_cache_shutdown()) the nfsd_file kmem_cache is destroyed
> while those leaked objects are still live:
> 
>   BUG nfsd_file: Objects remaining on __kmem_cache_shutdown()
>   WARNING: CPU: 3 PID: 849411 at mm/slub.c:1126 __slab_err
>    __kmem_cache_shutdown
>    kmem_cache_destroy
>    nfsd_file_cache_shutdown [nfsd]
>    nfsd_destroy_serv [nfsd]
>    nfsd_svc [nfsd]
>    nfsd_nl_threads_set_doit [nfsd]
> 
> which panics the box under panic_on_warn. Confirmed on the vmcore: the
> remaining nfsd_file objects are the nf_src/nf_dst of two in-flight async
> nfsd4_copy structures (cp_clp == NULL, refcount 1, files still held).
> 
> Release the files in nfsd4_stop_copy() and make release_copy_files()
> idempotent (NULL the pointers after the put) so it stays safe no matter
> which path - stop or reaper - reaches the copy.
> 
> This is the leak-fix portion of mainline commit 3daab3112f039 ("nfsd:
> cancel async COPY operations when admin revokes filesystem state"); the
> larger admin-revoke feature from that commit is not backported.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-134677
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  fs/nfsd/nfs4proc.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 47dba8382ede..9b9ec637ff41 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1428,6 +1428,8 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
>  	kfree(copy);
>  }
>  
> +static void release_copy_files(struct nfsd4_copy *copy);
> +
>  static void nfsd4_stop_copy(struct nfsd4_copy *copy)
>  {
>  	trace_nfsd_copy_async_cancel(copy);
> @@ -1436,6 +1438,15 @@ static void nfsd4_stop_copy(struct nfsd4_copy *copy)
>  		copy->nfserr = nfs_ok;
>  		set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
>  	}
> +
> +	/*
> +	 * The copy was removed from async_copies before this function
> +	 * was called, so the reaper cannot clean it up. Release files
> +	 * here regardless of who won the STOPPED race. If the thread
> +	 * set STOPPED, it has finished using the files. If STOPPED
> +	 * was set here, kthread_stop() waited for the thread to exit.
> +	 */
> +	release_copy_files(copy);
>  	nfs4_put_copy(copy);
>  }
>  
> @@ -1871,10 +1882,14 @@ static void dup_copy_fields(struct nfsd4_copy *src, struct nfsd4_copy *dst)
>  
>  static void release_copy_files(struct nfsd4_copy *copy)
>  {
> -	if (copy->nf_src)
> +	if (copy->nf_src) {
>  		nfsd_file_put(copy->nf_src);
> -	if (copy->nf_dst)
> +		copy->nf_src = NULL;
> +	}
> +	if (copy->nf_dst) {
>  		nfsd_file_put(copy->nf_dst);
> +		copy->nf_dst = NULL;
> +	}
>  }
>  
>  static void cleanup_async_copy(struct nfsd4_copy *copy)

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.