Commit e6f7e148 authored by NeilBrown's avatar NeilBrown Committed by Anna Schumaker
Browse files

nfs_localio: simplify interface to nfsd for getting nfsd_file 



The nfsd_localio_operations structure contains nfsd_file_get() to get a
reference to an nfsd_file.  This is only used in one place, where
nfsd_open_local_fh() is also used.

This patch combines the two, calling nfsd_open_local_fh() passing a
pointer to where the nfsd_file pointer might be stored.  If there is a
pointer there an nfsd_file_get() can get a reference, that reference is
returned.  If not a new nfsd_file is acquired, stored at the pointer,
and returned.  When we store a reference we also increase the refcount
on the net, as that refcount is decrements when we clear the stored
pointer.

We now get an extra reference *before* storing the new nfsd_file at the
given location.  This avoids possible races with the nfsd_file being
freed before the final reference can be taken.

This patch moves the rcu_dereference() needed after fetching from
ro_file or rw_file into the nfsd code where the 'struct nfs_file' is
fully defined.  This avoids an error reported by older versions of gcc
such as gcc-8 which complain about rcu_dereference() use in contexts
where the structure (which will supposedly be accessed) is not fully
defined.

Reported-by: default avatarPali Rohár <pali@kernel.org>
Reported-by: default avatarVincent Mailhol <mailhol.vincent@wanadoo.fr>
Fixes: 86e00412 ("nfs: cache all open LOCALIO nfsd_file(s) in client")
Signed-off-by: default avatarNeilBrown <neil@brown.name>
Signed-off-by: default avatarAnna Schumaker <anna.schumaker@oracle.com>
parent 77e82fb2

fs/nfs/localio.c

+6 −25
Original line number Diff line number Diff line
@@ -209,11 +209,6 @@ void nfs_local_probe_async(struct nfs_client *clp) 
}

EXPORT_SYMBOL_GPL(nfs_local_probe_async);



static inline struct nfsd_file *nfs_local_file_get(struct nfsd_file *nf)

{

	return nfs_to->nfsd_file_get_local(nf);

}



static inline void nfs_local_file_put(struct nfsd_file *nf)

{

	nfs_to_nfsd_file_put_local(nf);
@@ -228,12 +223,13 @@ static inline void nfs_local_file_put(struct nfsd_file *nf) 
static struct nfsd_file *

__nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,

		    struct nfs_fh *fh, struct nfs_file_localio *nfl,

		    struct nfsd_file __rcu **pnf,

		    const fmode_t mode)

{

	struct nfsd_file *localio;



	localio = nfs_open_local_fh(&clp->cl_uuid, clp->cl_rpcclient,

				    cred, fh, nfl, mode);

				    cred, fh, nfl, pnf, mode);

	if (IS_ERR(localio)) {

		int status = PTR_ERR(localio);

		trace_nfs_local_open_fh(fh, mode, status);
@@ -260,7 +256,7 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred, 
		  struct nfs_fh *fh, struct nfs_file_localio *nfl,

		  const fmode_t mode)

{

	struct nfsd_file *nf, *new, __rcu **pnf;

	struct nfsd_file *nf, __rcu **pnf;



	if (!nfs_server_is_local(clp))

		return NULL;
@@ -272,24 +268,9 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred, 
	else

		pnf = &nfl->ro_file;



	new = NULL;

	rcu_read_lock();

	nf = rcu_dereference(*pnf);

	if (!nf) {

		rcu_read_unlock();

		new = __nfs_local_open_fh(clp, cred, fh, nfl, mode);

		if (IS_ERR(new))

	nf = __nfs_local_open_fh(clp, cred, fh, nfl, pnf, mode);

	if (IS_ERR(nf))

		return NULL;

		rcu_read_lock();

		/* try to swap in the pointer */

		nf = unrcu_pointer(cmpxchg(pnf, NULL, RCU_INITIALIZER(new)));

		if (!nf)

			swap(nf, new);

	}

	nf = nfs_local_file_get(nf);

	rcu_read_unlock();

	if (new)

		nfs_to_nfsd_file_put_local(new);

	return nf;

}

EXPORT_SYMBOL_GPL(nfs_local_open_fh);

fs/nfs_common/nfslocalio.c

+2 −1
Original line number Diff line number Diff line
@@ -237,6 +237,7 @@ static void nfs_uuid_add_file(nfs_uuid_t *nfs_uuid, struct nfs_file_localio *nfl 
struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *uuid,

		   struct rpc_clnt *rpc_clnt, const struct cred *cred,

		   const struct nfs_fh *nfs_fh, struct nfs_file_localio *nfl,

		   struct nfsd_file __rcu **pnf,

		   const fmode_t fmode)

{

	struct net *net;
@@ -261,7 +262,7 @@ struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *uuid, 
	rcu_read_unlock();

	/* We have an implied reference to net thanks to nfsd_net_try_get */

	localio = nfs_to->nfsd_open_local_fh(net, uuid->dom, rpc_clnt,

					     cred, nfs_fh, fmode);

					     cred, nfs_fh, pnf, fmode);

	nfs_to_nfsd_net_put(net);

	if (!IS_ERR(localio))

		nfs_uuid_add_file(uuid, nfl);

fs/nfsd/localio.c

+47 −18
Original line number Diff line number Diff line
@@ -24,20 +24,6 @@ 
#include "filecache.h"

#include "cache.h"



static const struct nfsd_localio_operations nfsd_localio_ops = {

	.nfsd_net_try_get  = nfsd_net_try_get,

	.nfsd_net_put  = nfsd_net_put,

	.nfsd_open_local_fh = nfsd_open_local_fh,

	.nfsd_file_put_local = nfsd_file_put_local,

	.nfsd_file_get_local = nfsd_file_get_local,

	.nfsd_file_file = nfsd_file_file,

};



void nfsd_localio_ops_init(void)

{

	nfs_to = &nfsd_localio_ops;

}



/**

 * nfsd_open_local_fh - lookup a local filehandle @nfs_fh and map to nfsd_file

 *
@@ -46,6 +32,7 @@ void nfsd_localio_ops_init(void) 
 * @rpc_clnt: rpc_clnt that the client established

 * @cred: cred that the client established

 * @nfs_fh: filehandle to lookup

 * @nfp: place to find the nfsd_file, or store it if it was non-NULL

 * @fmode: fmode_t to use for open

 *

 * This function maps a local fh to a path on a local filesystem.
@@ -56,10 +43,11 @@ void nfsd_localio_ops_init(void) 
 * set. Caller (NFS client) is responsible for calling nfsd_net_put and

 * nfsd_file_put (via nfs_to_nfsd_file_put_local).

 */

struct nfsd_file *

static struct nfsd_file *

nfsd_open_local_fh(struct net *net, struct auth_domain *dom,

		   struct rpc_clnt *rpc_clnt, const struct cred *cred,

		   const struct nfs_fh *nfs_fh, const fmode_t fmode)

		   const struct nfs_fh *nfs_fh, struct nfsd_file __rcu **pnf,

		   const fmode_t fmode)

{

	int mayflags = NFSD_MAY_LOCALIO;

	struct svc_cred rq_cred;
@@ -73,6 +61,12 @@ nfsd_open_local_fh(struct net *net, struct auth_domain *dom, 
	if (!nfsd_net_try_get(net))

		return ERR_PTR(-ENXIO);



	rcu_read_lock();

	localio = nfsd_file_get(rcu_dereference(*pnf));

	rcu_read_unlock();

	if (localio)

		return localio;



	/* nfs_fh -> svc_fh */

	fh_init(&fh, NFS4_FHSIZE);

	fh.fh_handle.fh_size = nfs_fh->size;
@@ -94,12 +88,47 @@ nfsd_open_local_fh(struct net *net, struct auth_domain *dom, 
	if (rq_cred.cr_group_info)

		put_group_info(rq_cred.cr_group_info);



	if (IS_ERR(localio))

	if (!IS_ERR(localio)) {

		struct nfsd_file *new;

		if (!nfsd_net_try_get(net)) {

			nfsd_file_put(localio);

			nfsd_net_put(net);

			return ERR_PTR(-ENXIO);

		}

		nfsd_file_get(localio);

	again:

		new = unrcu_pointer(cmpxchg(pnf, NULL, RCU_INITIALIZER(localio)));

		if (new) {

			/* Some other thread installed an nfsd_file */

			if (nfsd_file_get(new) == NULL)

				goto again;

			/*

			 * Drop the ref we were going to install and the

			 * one we were going to return.

			 */

			nfsd_file_put(localio);

			nfsd_file_put(localio);

			localio = new;

		}

	} else

		nfsd_net_put(net);



	return localio;

}

EXPORT_SYMBOL_GPL(nfsd_open_local_fh);



static const struct nfsd_localio_operations nfsd_localio_ops = {

	.nfsd_net_try_get  = nfsd_net_try_get,

	.nfsd_net_put  = nfsd_net_put,

	.nfsd_open_local_fh = nfsd_open_local_fh,

	.nfsd_file_put_local = nfsd_file_put_local,

	.nfsd_file_get_local = nfsd_file_get_local,

	.nfsd_file_file = nfsd_file_file,

};



void nfsd_localio_ops_init(void)

{

	nfs_to = &nfsd_localio_ops;

}



/*

 * UUID_IS_LOCAL XDR functions