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

nfs_localio: change nfsd_file_put_local() to take a pointer to __rcu pointer 



Instead of calling xchg() and unrcu_pointer() before
nfsd_file_put_local(), we now pass pointer to the __rcu pointer and call
xchg() and unrcu_pointer() inside that function.

Where unrcu_pointer() is currently called the internals of "struct
nfsd_file" are not known and that causes older compilers such as gcc-8
to complain.

In some cases we have a __kernel (aka normal) pointer not an __rcu
pointer so we need to cast it to __rcu first.  This is strictly a
weakening so no information is lost.  Somewhat surprisingly, this cast
is accepted by gcc-8.

This has the pleasing result that the cmpxchg() which sets ro_file and
rw_file, and also the xchg() which clears them, are both now in the nfsd
code.

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 21fb4403

fs/nfs/localio.c

+9 −2
Original line number Diff line number Diff line
@@ -209,9 +209,16 @@ void nfs_local_probe_async(struct nfs_client *clp) 
}

EXPORT_SYMBOL_GPL(nfs_local_probe_async);



static inline void nfs_local_file_put(struct nfsd_file *nf)

static inline void nfs_local_file_put(struct nfsd_file *localio)

{

	nfs_to_nfsd_file_put_local(nf);

	/* nfs_to_nfsd_file_put_local() expects an __rcu pointer

	 * but we have a __kernel pointer.  It is always safe

	 * to cast a __kernel pointer to an __rcu pointer

	 * because the cast only weakens what is known about the pointer.

	 */

	struct nfsd_file __rcu *nf = (struct nfsd_file __rcu*) localio;



	nfs_to_nfsd_file_put_local(&nf);

}



/*

fs/nfs_common/nfslocalio.c

+6 −18
Original line number Diff line number Diff line
@@ -170,9 +170,6 @@ static bool nfs_uuid_put(nfs_uuid_t *nfs_uuid) 
	while ((nfl = list_first_entry_or_null(&nfs_uuid->files,

					       struct nfs_file_localio,

					       list)) != NULL) {

		struct nfsd_file *ro_nf;

		struct nfsd_file *rw_nf;



		/* If nfs_uuid is already NULL, nfs_close_local_fh is

		 * closing and we must wait, else we unlink and close.

		 */
@@ -189,17 +186,14 @@ static bool nfs_uuid_put(nfs_uuid_t *nfs_uuid) 
			continue;

		}



		ro_nf = unrcu_pointer(xchg(&nfl->ro_file, NULL));

		rw_nf = unrcu_pointer(xchg(&nfl->rw_file, NULL));



		/* Remove nfl from nfs_uuid->files list */

		list_del_init(&nfl->list);

		spin_unlock(&nfs_uuid->lock);

		if (ro_nf)

			nfs_to_nfsd_file_put_local(ro_nf);

		if (rw_nf)

			nfs_to_nfsd_file_put_local(rw_nf);



		nfs_to_nfsd_file_put_local(&nfl->ro_file);

		nfs_to_nfsd_file_put_local(&nfl->rw_file);

		cond_resched();



		spin_lock(&nfs_uuid->lock);

		/* Now we can allow racing nfs_close_local_fh() to

		 * skip the locking.
@@ -303,8 +297,6 @@ EXPORT_SYMBOL_GPL(nfs_open_local_fh); 


void nfs_close_local_fh(struct nfs_file_localio *nfl)

{

	struct nfsd_file *ro_nf;

	struct nfsd_file *rw_nf;

	nfs_uuid_t *nfs_uuid;



	rcu_read_lock();
@@ -337,12 +329,8 @@ void nfs_close_local_fh(struct nfs_file_localio *nfl) 
	spin_unlock(&nfs_uuid->lock);

	rcu_read_unlock();



	ro_nf = unrcu_pointer(xchg(&nfl->ro_file, NULL));

	rw_nf = unrcu_pointer(xchg(&nfl->rw_file, NULL));

	if (ro_nf)

		nfs_to_nfsd_file_put_local(ro_nf);

	if (rw_nf)

		nfs_to_nfsd_file_put_local(rw_nf);

	nfs_to_nfsd_file_put_local(&nfl->ro_file);

	nfs_to_nfsd_file_put_local(&nfl->rw_file);



	/* Remove nfl from nfs_uuid->files list and signal nfs_uuid_put()

	 * that we are done.  The moment we drop the spinlock the

fs/nfsd/filecache.c

+8 −3
Original line number Diff line number Diff line
@@ -378,11 +378,16 @@ nfsd_file_put(struct nfsd_file *nf) 
 * the reference of the nfsd_file.

 */

struct net *

nfsd_file_put_local(struct nfsd_file *nf)

nfsd_file_put_local(struct nfsd_file __rcu **pnf)

{

	struct net *net = nf->nf_net;

	struct nfsd_file *nf;

	struct net *net = NULL;



	nf = unrcu_pointer(xchg(pnf, NULL));

	if (nf) {

		net = nf->nf_net;

		nfsd_file_put(nf);

	}

	return net;

}

fs/nfsd/filecache.h

+1 −1
Original line number Diff line number Diff line
@@ -62,7 +62,7 @@ void nfsd_file_cache_shutdown(void); 
int nfsd_file_cache_start_net(struct net *net);

void nfsd_file_cache_shutdown_net(struct net *net);

void nfsd_file_put(struct nfsd_file *nf);

struct net *nfsd_file_put_local(struct nfsd_file *nf);

struct net *nfsd_file_put_local(struct nfsd_file __rcu **nf);

struct nfsd_file *nfsd_file_get_local(struct nfsd_file *nf);

struct nfsd_file *nfsd_file_get(struct nfsd_file *nf);

struct file *nfsd_file_file(struct nfsd_file *nf);

include/linux/nfslocalio.h

+12 −11
Original line number Diff line number Diff line
@@ -50,10 +50,6 @@ void nfs_localio_invalidate_clients(struct list_head *nn_local_clients, 
				    spinlock_t *nn_local_clients_lock);



/* localio needs to map filehandle -> struct nfsd_file */

extern struct nfsd_file *

nfsd_open_local_fh(struct net *, struct auth_domain *, struct rpc_clnt *,

		   const struct cred *, const struct nfs_fh *,

		   const fmode_t) __must_hold(rcu);

void nfs_close_local_fh(struct nfs_file_localio *);



struct nfsd_localio_operations {
@@ -64,8 +60,9 @@ struct nfsd_localio_operations { 
						struct rpc_clnt *,

						const struct cred *,

						const struct nfs_fh *,

						struct nfsd_file __rcu **pnf,

						const fmode_t);

	struct net *(*nfsd_file_put_local)(struct nfsd_file *);

	struct net *(*nfsd_file_put_local)(struct nfsd_file __rcu **);

	struct nfsd_file *(*nfsd_file_get_local)(struct nfsd_file *);

	struct file *(*nfsd_file_file)(struct nfsd_file *);

} ____cacheline_aligned;
@@ -76,6 +73,7 @@ extern const struct nfsd_localio_operations *nfs_to; 
struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *,

		   struct rpc_clnt *, const struct cred *,

		   const struct nfs_fh *, struct nfs_file_localio *,

		   struct nfsd_file __rcu **pnf,

		   const fmode_t);



static inline void nfs_to_nfsd_net_put(struct net *net)
@@ -90,15 +88,18 @@ static inline void nfs_to_nfsd_net_put(struct net *net) 
	rcu_read_unlock();

}



static inline void nfs_to_nfsd_file_put_local(struct nfsd_file *localio)

static inline void nfs_to_nfsd_file_put_local(struct nfsd_file __rcu **localio)

{

	/*

	 * Must not hold RCU otherwise nfsd_file_put() can easily trigger:

	 * "Voluntary context switch within RCU read-side critical section!"

	 * by scheduling deep in underlying filesystem (e.g. XFS).

	 * Either *localio must be guaranteed to be non-NULL, or caller

	 * must prevent nfsd shutdown from completing as nfs_close_local_fh()

	 * does by blocking the nfs_uuid from being finally put.

	 */

	struct net *net = nfs_to->nfsd_file_put_local(localio);

	struct net *net;



	net = nfs_to->nfsd_file_put_local(localio);



	if (net)

		nfs_to_nfsd_net_put(net);

}