Re: [PATCH v13 13/19] nfs: add localio support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 24 Aug 2024, Mike Snitzer wrote:
> +/*
> + * nfs_local_disable - disable local i/o for an nfs_client
> + */
> +void nfs_local_disable(struct nfs_client *clp)
> +{
> +	if (test_and_clear_bit(NFS_CS_LOCAL_IO, &clp->cl_flags)) {

This use of test_and_clear_bit() led me to think about the atomicity
requirements here.

If this were called (on a different CPU) between the
nfs_server_is_local() test in nfs_local_file_open() and the
nfs_local_open_fh() call which follows, then nfs_to.nfsd_open_local_fh()
could be passed a NULL client which wouldn't end well.

Maybe we need an rcu-protected auth_domain_tryget() in
nfsd_local_open_fh(), and the auth_domain_put() below need to be behind
call_rcu().

Or something...

> +		trace_nfs_local_disable(clp);
> +		clp->cl_nfssvc_net = NULL;
> +		if (clp->cl_nfssvc_dom) {
> +			auth_domain_put(clp->cl_nfssvc_dom);
> +			clp->cl_nfssvc_dom = NULL;
> +		}
> +	}
> +}


> +
> +struct nfsd_file *
> +nfs_local_file_open(struct nfs_client *clp, const struct cred *cred,
> +		    struct nfs_fh *fh, struct nfs_open_context *ctx)
> +{
> +	struct nfsd_file *nf;
> +
> +	if (!nfs_server_is_local(clp))
> +		return NULL;
> +
> +	nf = nfs_local_open_fh(clp, cred, fh, ctx->mode);
> +	if (IS_ERR(nf))
> +		return NULL;
> +
> +	return nf;
> +}

It isn't clear to be what nfs_local_file_open() adds any value over
nfs_local_open_fh.  The nfs_server_is_local() test is cheap and can
safely be in nfs_local_open_fh(), and the current callers of
nfs_local_file_open() can simple pass ctx->mode to _fh instead of
passing ctx to _open.

Thanks,
NeilBrown




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux