Skip to content

Commit b33f7de

Browse files
Mike SnitzerAnna Schumaker
authored andcommitted
nfsd: rename nfsd_serv_ prefixed methods and variables with nfsd_net_
Also update Documentation/filesystems/nfs/localio.rst accordingly and reduce the technical documentation debt that was previously captured in that document. Signed-off-by: Mike Snitzer <[email protected]> Reviewed-by: Jeff Layton <[email protected]> Acked-by: Chuck Lever <[email protected]> Signed-off-by: Anna Schumaker <[email protected]>
1 parent 3997249 commit b33f7de

File tree

7 files changed

+66
-92
lines changed

7 files changed

+66
-92
lines changed

Documentation/filesystems/nfs/localio.rst

Lines changed: 27 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -218,64 +218,30 @@ NFS Client and Server Interlock
218218
===============================
219219

220220
LOCALIO provides the nfs_uuid_t object and associated interfaces to
221-
allow proper network namespace (net-ns) and NFSD object refcounting:
222-
223-
We don't want to keep a long-term counted reference on each NFSD's
224-
net-ns in the client because that prevents a server container from
225-
completely shutting down.
226-
227-
So we avoid taking a reference at all and rely on the per-cpu
228-
reference to the server (detailed below) being sufficient to keep
229-
the net-ns active. This involves allowing the NFSD's net-ns exit
230-
code to iterate all active clients and clear their ->net pointers
231-
(which are needed to find the per-cpu-refcount for the nfsd_serv).
232-
233-
Details:
234-
235-
- Embed nfs_uuid_t in nfs_client. nfs_uuid_t provides a list_head
236-
that can be used to find the client. It does add the 16-byte
237-
uuid_t to nfs_client so it is bigger than needed (given that
238-
uuid_t is only used during the initial NFS client and server
239-
LOCALIO handshake to determine if they are local to each other).
240-
If that is really a problem we can find a fix.
241-
242-
- When the nfs server confirms that the uuid_t is local, it moves
243-
the nfs_uuid_t onto a per-net-ns list in NFSD's nfsd_net.
244-
245-
- When each server's net-ns is shutting down - in a "pre_exit"
246-
handler, all these nfs_uuid_t have their ->net cleared. There is
247-
an rcu_synchronize() call between pre_exit() handlers and exit()
248-
handlers so any caller that sees nfs_uuid_t ->net as not NULL can
249-
safely manage the per-cpu-refcount for nfsd_serv.
250-
251-
- The client's nfs_uuid_t is passed to nfsd_open_local_fh() so it
252-
can safely dereference ->net in a private rcu_read_lock() section
253-
to allow safe access to the associated nfsd_net and nfsd_serv.
254-
255-
So LOCALIO required the introduction and use of NFSD's percpu_ref to
256-
interlock nfsd_destroy_serv() and nfsd_open_local_fh(), to ensure each
257-
nn->nfsd_serv is not destroyed while in use by nfsd_open_local_fh(), and
221+
allow proper network namespace (net-ns) and NFSD object refcounting.
222+
223+
LOCALIO required the introduction and use of NFSD's percpu nfsd_net_ref
224+
to interlock nfsd_shutdown_net() and nfsd_open_local_fh(), to ensure
225+
each net-ns is not destroyed while in use by nfsd_open_local_fh(), and
258226
warrants a more detailed explanation:
259227

260-
nfsd_open_local_fh() uses nfsd_serv_try_get() before opening its
228+
nfsd_open_local_fh() uses nfsd_net_try_get() before opening its
261229
nfsd_file handle and then the caller (NFS client) must drop the
262-
reference for the nfsd_file and associated nn->nfsd_serv using
263-
nfs_file_put_local() once it has completed its IO.
230+
reference for the nfsd_file and associated net-ns using
231+
nfsd_file_put_local() once it has completed its IO.
264232

265233
This interlock working relies heavily on nfsd_open_local_fh() being
266234
afforded the ability to safely deal with the possibility that the
267235
NFSD's net-ns (and nfsd_net by association) may have been destroyed
268-
by nfsd_destroy_serv() via nfsd_shutdown_net() -- which is only
269-
possible given the nfs_uuid_t ->net pointer managemenet detailed
270-
above.
271-
272-
All told, this elaborate interlock of the NFS client and server has been
273-
verified to fix an easy to hit crash that would occur if an NFSD
274-
instance running in a container, with a LOCALIO client mounted, is
275-
shutdown. Upon restart of the container and associated NFSD the client
276-
would go on to crash due to NULL pointer dereference that occurred due
277-
to the LOCALIO client's attempting to nfsd_open_local_fh(), using
278-
nn->nfsd_serv, without having a proper reference on nn->nfsd_serv.
236+
by nfsd_destroy_serv() via nfsd_shutdown_net().
237+
238+
This interlock of the NFS client and server has been verified to fix an
239+
easy to hit crash that would occur if an NFSD instance running in a
240+
container, with a LOCALIO client mounted, is shutdown. Upon restart of
241+
the container and associated NFSD, the client would go on to crash due
242+
to NULL pointer dereference that occurred due to the LOCALIO client's
243+
attempting to nfsd_open_local_fh() without having a proper reference on
244+
NFSD's net-ns.
279245

280246
NFS Client issues IO instead of Server
281247
======================================
@@ -308,16 +274,19 @@ fs/nfs/localio.c:nfs_local_commit().
308274

309275
With normal NFS that makes use of RPC to issue IO to the server, if an
310276
application uses O_DIRECT the NFS client will bypass the pagecache but
311-
the NFS server will not. Because the NFS server's use of buffered IO
312-
affords applications to be less precise with their alignment when
313-
issuing IO to the NFS client. LOCALIO can be configured to use O_DIRECT
314-
semantics by setting the 'localio_O_DIRECT_semantics' nfs module
277+
the NFS server will not. The NFS server's use of buffered IO affords
278+
applications to be less precise with their alignment when issuing IO to
279+
the NFS client. But if all applications properly align their IO, LOCALIO
280+
can be configured to use end-to-end O_DIRECT semantics from the NFS
281+
client to the underlying local filesystem, that it is sharing with
282+
the NFS server, by setting the 'localio_O_DIRECT_semantics' nfs module
315283
parameter to Y, e.g.:
316284

317-
echo Y > /sys/module/nfs/parameters/localio_O_DIRECT_semantics
285+
echo Y > /sys/module/nfs/parameters/localio_O_DIRECT_semantics
318286

319-
Once enabled, it will cause LOCALIO to use O_DIRECT semantics (this may
320-
cause IO to fail if applications do not properly align their IO).
287+
Once enabled, it will cause LOCALIO to use end-to-end O_DIRECT semantics
288+
(but again, this may cause IO to fail if applications do not properly
289+
align their IO).
321290

322291
Security
323292
========

fs/nfs_common/nfslocalio.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ static void nfs_uuid_add_file(nfs_uuid_t *nfs_uuid, struct nfs_file_localio *nfl
159159
spin_unlock(&nfs_uuid_lock);
160160
}
161161

162+
/*
163+
* Caller is responsible for calling nfsd_net_put and
164+
* nfsd_file_put (via nfs_to_nfsd_file_put_local).
165+
*/
162166
struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *uuid,
163167
struct rpc_clnt *rpc_clnt, const struct cred *cred,
164168
const struct nfs_fh *nfs_fh, struct nfs_file_localio *nfl,
@@ -171,20 +175,20 @@ struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *uuid,
171175
* Not running in nfsd context, so must safely get reference on nfsd_serv.
172176
* But the server may already be shutting down, if so disallow new localio.
173177
* uuid->net is NOT a counted reference, but rcu_read_lock() ensures that
174-
* if uuid->net is not NULL, then calling nfsd_serv_try_get() is safe
178+
* if uuid->net is not NULL, then calling nfsd_net_try_get() is safe
175179
* and if it succeeds we will have an implied reference to the net.
176180
*
177181
* Otherwise NFS may not have ref on NFSD and therefore cannot safely
178182
* make 'nfs_to' calls.
179183
*/
180184
rcu_read_lock();
181185
net = rcu_dereference(uuid->net);
182-
if (!net || !nfs_to->nfsd_serv_try_get(net)) {
186+
if (!net || !nfs_to->nfsd_net_try_get(net)) {
183187
rcu_read_unlock();
184188
return ERR_PTR(-ENXIO);
185189
}
186190
rcu_read_unlock();
187-
/* We have an implied reference to net thanks to nfsd_serv_try_get */
191+
/* We have an implied reference to net thanks to nfsd_net_try_get */
188192
localio = nfs_to->nfsd_open_local_fh(net, uuid->dom, rpc_clnt,
189193
cred, nfs_fh, fmode);
190194
if (IS_ERR(localio))

fs/nfsd/filecache.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ nfsd_file_put(struct nfsd_file *nf)
391391
}
392392

393393
/**
394-
* nfsd_file_put_local - put nfsd_file reference and arm nfsd_serv_put in caller
394+
* nfsd_file_put_local - put nfsd_file reference and arm nfsd_net_put in caller
395395
* @nf: nfsd_file of which to put the reference
396396
*
397397
* First save the associated net to return to caller, then put

fs/nfsd/localio.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
#include "cache.h"
2626

2727
static const struct nfsd_localio_operations nfsd_localio_ops = {
28-
.nfsd_serv_try_get = nfsd_serv_try_get,
29-
.nfsd_serv_put = nfsd_serv_put,
28+
.nfsd_net_try_get = nfsd_net_try_get,
29+
.nfsd_net_put = nfsd_net_put,
3030
.nfsd_open_local_fh = nfsd_open_local_fh,
3131
.nfsd_file_put_local = nfsd_file_put_local,
3232
.nfsd_file_get = nfsd_file_get,

fs/nfsd/netns.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,10 @@ struct nfsd_net {
140140

141141
struct svc_info nfsd_info;
142142
#define nfsd_serv nfsd_info.serv
143-
struct percpu_ref nfsd_serv_ref;
144-
struct completion nfsd_serv_confirm_done;
145-
struct completion nfsd_serv_free_done;
143+
144+
struct percpu_ref nfsd_net_ref;
145+
struct completion nfsd_net_confirm_done;
146+
struct completion nfsd_net_free_done;
146147

147148
/*
148149
* clientid and stateid data for construction of net unique COPY
@@ -229,8 +230,8 @@ struct nfsd_net {
229230
extern bool nfsd_support_version(int vers);
230231
extern unsigned int nfsd_net_id;
231232

232-
bool nfsd_serv_try_get(struct net *net);
233-
void nfsd_serv_put(struct net *net);
233+
bool nfsd_net_try_get(struct net *net);
234+
void nfsd_net_put(struct net *net);
234235

235236
void nfsd_copy_write_verifier(__be32 verf[2], struct nfsd_net *nn);
236237
void nfsd_reset_write_verifier(struct nfsd_net *nn);

fs/nfsd/nfssvc.c

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -214,32 +214,32 @@ int nfsd_minorversion(struct nfsd_net *nn, u32 minorversion, enum vers_op change
214214
return 0;
215215
}
216216

217-
bool nfsd_serv_try_get(struct net *net) __must_hold(rcu)
217+
bool nfsd_net_try_get(struct net *net) __must_hold(rcu)
218218
{
219219
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
220220

221-
return (nn && percpu_ref_tryget_live(&nn->nfsd_serv_ref));
221+
return (nn && percpu_ref_tryget_live(&nn->nfsd_net_ref));
222222
}
223223

224-
void nfsd_serv_put(struct net *net) __must_hold(rcu)
224+
void nfsd_net_put(struct net *net) __must_hold(rcu)
225225
{
226226
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
227227

228-
percpu_ref_put(&nn->nfsd_serv_ref);
228+
percpu_ref_put(&nn->nfsd_net_ref);
229229
}
230230

231-
static void nfsd_serv_done(struct percpu_ref *ref)
231+
static void nfsd_net_done(struct percpu_ref *ref)
232232
{
233-
struct nfsd_net *nn = container_of(ref, struct nfsd_net, nfsd_serv_ref);
233+
struct nfsd_net *nn = container_of(ref, struct nfsd_net, nfsd_net_ref);
234234

235-
complete(&nn->nfsd_serv_confirm_done);
235+
complete(&nn->nfsd_net_confirm_done);
236236
}
237237

238-
static void nfsd_serv_free(struct percpu_ref *ref)
238+
static void nfsd_net_free(struct percpu_ref *ref)
239239
{
240-
struct nfsd_net *nn = container_of(ref, struct nfsd_net, nfsd_serv_ref);
240+
struct nfsd_net *nn = container_of(ref, struct nfsd_net, nfsd_net_ref);
241241

242-
complete(&nn->nfsd_serv_free_done);
242+
complete(&nn->nfsd_net_free_done);
243243
}
244244

245245
/*
@@ -437,8 +437,8 @@ static void nfsd_shutdown_net(struct net *net)
437437
if (!nn->nfsd_net_up)
438438
return;
439439

440-
percpu_ref_kill_and_confirm(&nn->nfsd_serv_ref, nfsd_serv_done);
441-
wait_for_completion(&nn->nfsd_serv_confirm_done);
440+
percpu_ref_kill_and_confirm(&nn->nfsd_net_ref, nfsd_net_done);
441+
wait_for_completion(&nn->nfsd_net_confirm_done);
442442

443443
nfsd_export_flush(net);
444444
nfs4_state_shutdown_net(net);
@@ -449,8 +449,8 @@ static void nfsd_shutdown_net(struct net *net)
449449
nn->lockd_up = false;
450450
}
451451

452-
wait_for_completion(&nn->nfsd_serv_free_done);
453-
percpu_ref_exit(&nn->nfsd_serv_ref);
452+
wait_for_completion(&nn->nfsd_net_free_done);
453+
percpu_ref_exit(&nn->nfsd_net_ref);
454454

455455
nn->nfsd_net_up = false;
456456
nfsd_shutdown_generic();
@@ -654,12 +654,12 @@ int nfsd_create_serv(struct net *net)
654654
if (nn->nfsd_serv)
655655
return 0;
656656

657-
error = percpu_ref_init(&nn->nfsd_serv_ref, nfsd_serv_free,
657+
error = percpu_ref_init(&nn->nfsd_net_ref, nfsd_net_free,
658658
0, GFP_KERNEL);
659659
if (error)
660660
return error;
661-
init_completion(&nn->nfsd_serv_free_done);
662-
init_completion(&nn->nfsd_serv_confirm_done);
661+
init_completion(&nn->nfsd_net_free_done);
662+
init_completion(&nn->nfsd_net_confirm_done);
663663

664664
if (nfsd_max_blksize == 0)
665665
nfsd_max_blksize = nfsd_get_default_max_blksize();

include/linux/nfslocalio.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ nfsd_open_local_fh(struct net *, struct auth_domain *, struct rpc_clnt *,
5252
void nfs_close_local_fh(struct nfs_file_localio *);
5353

5454
struct nfsd_localio_operations {
55-
bool (*nfsd_serv_try_get)(struct net *);
56-
void (*nfsd_serv_put)(struct net *);
55+
bool (*nfsd_net_try_get)(struct net *);
56+
void (*nfsd_net_put)(struct net *);
5757
struct nfsd_file *(*nfsd_open_local_fh)(struct net *,
5858
struct auth_domain *,
5959
struct rpc_clnt *,
@@ -77,12 +77,12 @@ struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *,
7777
static inline void nfs_to_nfsd_net_put(struct net *net)
7878
{
7979
/*
80-
* Once reference to nfsd_serv is dropped, NFSD could be
81-
* unloaded, so ensure safe return from nfsd_file_put_local()
82-
* by always taking RCU.
80+
* Once reference to net (and associated nfsd_serv) is dropped, NFSD
81+
* could be unloaded, so ensure safe return from nfsd_net_put() by
82+
* always taking RCU.
8383
*/
8484
rcu_read_lock();
85-
nfs_to->nfsd_serv_put(net);
85+
nfs_to->nfsd_net_put(net);
8686
rcu_read_unlock();
8787
}
8888

0 commit comments

Comments
 (0)