Re: [PATCH v13 09/19] nfs_common: add NFS LOCALIO auxiliary protocol enablement

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

 



On Sat, 24 Aug 2024, Mike Snitzer wrote:
> 
> Also, expose localio's required nfsd symbols to NFS client:
> - Cache nfsd_open_local_fh symbol (defined in next commit) and other
>   required nfsd symbols in a globally accessible 'nfs_to'
>   nfs_to_nfsd_t struct.

I'm not thrilled with the mechanism for getting these symbols.

I'd rather nfsd passed the symbols to nfs_uuid_is_local(), and it stored
them somewhere that nfs can see them.  No need for reference counting
etc.  If nfs/localio holds an auth_domain, then it implicitly holds a
reference to the nfsd module and the functions cannot disappear.

I would created an 'nfs_localio_operations' structure which is defined
in nfsd as a constant.
The address of this is passed to nfs_uud_is_local() and that address
is stored in nfs_to if it doesn't already have the correct value.

So no need for symbol_request() or symbol_put().


> +
> +DEFINE_MUTEX(nfs_uuid_mutex);

This doesn't need to be a mutex - a spinlock is sufficient.

> +
> +bool nfs_uuid_is_local(const uuid_t *uuid, struct net *net, struct auth_domain *dom)
> +{
> +	bool is_local = false;
> +	nfs_uuid_t *nfs_uuid;
> +
> +	rcu_read_lock();
> +	nfs_uuid = nfs_uuid_lookup(uuid);
> +	if (nfs_uuid) {
> +		is_local = true;
> +		nfs_uuid->net = net;

It looks odd that you don't take a reference to the net.
It is probably correct but a comment explaining why would help.
Is it that the dom implies a reference to the net?

> +		kref_get(&dom->ref);
> +		nfs_uuid->dom = dom;
> +	}
> +	rcu_read_unlock();
> +
> +	return is_local;
> +}
> +EXPORT_SYMBOL_GPL(nfs_uuid_is_local);

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