| Message ID | 87h7l6epmb.fsf@notabene.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] nfsd: report client confirmation status in "info" file | expand |
Hi Neil- > On Mar 19, 2021, at 6:38 PM, NeilBrown <neilb@suse.de> wrote: > > > mountd can now monitor clients appearing and disappearing in > /proc/fs/nfsd/clients, and will log these events, in liu of the logging > of mount/unmount events for NFSv3. > > Currently it cannot distinguish between unconfirmed clients (which might > be transient and totally uninteresting) and confirmed clients. > > So add a "status: " line which reports either "confirmed" or > "unconfirmed", and use fsnotify to report that the info file > has been modified. > > This requires a bit of infrastructure to keep the dentry for the "info" > file. There is no need to take a counted reference as the dentry must > remain around until the client is removed. > > Signed-off-by: NeilBrown <neilb@suse.de> > --- > fs/nfsd/nfs4state.c | 19 +++++++++++++++---- > fs/nfsd/nfsctl.c | 14 ++++++++------ > fs/nfsd/nfsd.h | 4 +++- > fs/nfsd/state.h | 4 ++++ > 4 files changed, 30 insertions(+), 11 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 97447a64bad0..ec1b2dd8fd45 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -43,6 +43,7 @@ > #include <linux/sunrpc/addr.h> > #include <linux/jhash.h> > #include <linux/string_helpers.h> > +#include <linux/fsnotify.h> > #include "xdr4.h" > #include "xdr4cb.h" > #include "vfs.h" > @@ -2352,6 +2353,10 @@ static int client_info_show(struct seq_file *m, void *v) > memcpy(&clid, &clp->cl_clientid, sizeof(clid)); > seq_printf(m, "clientid: 0x%llx\n", clid); > seq_printf(m, "address: \"%pISpc\"\n", (struct sockaddr *)&clp->cl_addr); > + if (test_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags)) > + seq_puts(m, "status: confirmed\n"); > + else > + seq_puts(m, "status: unconfirmed\n"); > seq_printf(m, "name: "); > seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len); > seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion); > @@ -2702,6 +2707,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, > int ret; > struct net *net = SVC_NET(rqstp); > struct nfsd_net *nn = net_generic(net, nfsd_net_id); > + struct dentry *dentries[ARRAY_SIZE(client_files)]; > > clp = alloc_client(name); > if (clp == NULL) > @@ -2721,9 +2727,11 @@ static struct nfs4_client *create_client(struct xdr_netobj name, > memcpy(&clp->cl_addr, sa, sizeof(struct sockaddr_storage)); > clp->cl_cb_session = NULL; > clp->net = net; > - clp->cl_nfsd_dentry = nfsd_client_mkdir(nn, &clp->cl_nfsdfs, > - clp->cl_clientid.cl_id - nn->clientid_base, > - client_files); > + clp->cl_nfsd_dentry = nfsd_client_mkdir( > + nn, &clp->cl_nfsdfs, > + clp->cl_clientid.cl_id - nn->clientid_base, > + client_files, dentries); > + clp->cl_nfsd_info_dentry = dentries[0]; > if (!clp->cl_nfsd_dentry) { > free_client(clp); > return NULL; > @@ -2798,7 +2806,10 @@ move_to_confirmed(struct nfs4_client *clp) > list_move(&clp->cl_idhash, &nn->conf_id_hashtbl[idhashval]); > rb_erase(&clp->cl_namenode, &nn->unconf_name_tree); > add_clp_to_name_tree(clp, &nn->conf_name_tree); > - set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags); > + if (!test_and_set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags) && > + clp->cl_nfsd_dentry && > + clp->cl_nfsd_info_dentry) > + fsnotify_dentry(clp->cl_nfsd_info_dentry, FS_MODIFY); I hit a "sleep while spin-locked" splat and bisected it to this commit. fsnotify() can allocate memory with GFP_KERNEL, so it can't be called while &nn->client_lock is held, unfortunately. > renew_client_locked(clp); > } > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > index ef86ed23af82..94ce1eabd9d1 100644 > --- a/fs/nfsd/nfsctl.c > +++ b/fs/nfsd/nfsctl.c > @@ -1266,7 +1266,8 @@ static void nfsdfs_remove_files(struct dentry *root) > /* XXX: cut'n'paste from simple_fill_super; figure out if we could share > * code instead. */ > static int nfsdfs_create_files(struct dentry *root, > - const struct tree_descr *files) > + const struct tree_descr *files, > + struct dentry **fdentries) > { > struct inode *dir = d_inode(root); > struct inode *inode; > @@ -1275,8 +1276,6 @@ static int nfsdfs_create_files(struct dentry *root, > > inode_lock(dir); > for (i = 0; files->name && files->name[0]; i++, files++) { > - if (!files->name) > - continue; > dentry = d_alloc_name(root, files->name); > if (!dentry) > goto out; > @@ -1290,6 +1289,8 @@ static int nfsdfs_create_files(struct dentry *root, > inode->i_private = __get_nfsdfs_client(dir); > d_add(dentry, inode); > fsnotify_create(dir, dentry); > + if (fdentries) > + fdentries[i] = dentry; > } > inode_unlock(dir); > return 0; > @@ -1301,8 +1302,9 @@ static int nfsdfs_create_files(struct dentry *root, > > /* on success, returns positive number unique to that client. */ > struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, > - struct nfsdfs_client *ncl, u32 id, > - const struct tree_descr *files) > + struct nfsdfs_client *ncl, u32 id, > + const struct tree_descr *files, > + struct dentry **fdentries) > { > struct dentry *dentry; > char name[11]; > @@ -1313,7 +1315,7 @@ struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, > dentry = nfsd_mkdir(nn->nfsd_client_dir, ncl, name); > if (IS_ERR(dentry)) /* XXX: tossing errors? */ > return NULL; > - ret = nfsdfs_create_files(dentry, files); > + ret = nfsdfs_create_files(dentry, files, fdentries); > if (ret) { > nfsd_client_rmdir(dentry); > return NULL; > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h > index 8bdc37aa2c2e..9aee72f65330 100644 > --- a/fs/nfsd/nfsd.h > +++ b/fs/nfsd/nfsd.h > @@ -107,7 +107,9 @@ struct nfsdfs_client { > > struct nfsdfs_client *get_nfsdfs_client(struct inode *); > struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, > - struct nfsdfs_client *ncl, u32 id, const struct tree_descr *); > + struct nfsdfs_client *ncl, u32 id, > + const struct tree_descr *, > + struct dentry **fdentries); > void nfsd_client_rmdir(struct dentry *dentry); > > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index 73deea353169..54cab651ac1d 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -371,6 +371,10 @@ struct nfs4_client { > > /* debugging info directory under nfsd/clients/ : */ > struct dentry *cl_nfsd_dentry; > + /* 'info' file within that directory. Ref is not counted, > + * but will remain valid iff cl_nfsd_dentry != NULL > + */ > + struct dentry *cl_nfsd_info_dentry; > > /* for nfs41 callbacks */ > /* We currently support a single back channel with a single slot */ > -- > 2.30.1 > -- Chuck Lever
> On May 18, 2022, at 10:45 AM, Chuck Lever III <chuck.lever@oracle.com> wrote: > > Hi Neil- > > >> On Mar 19, 2021, at 6:38 PM, NeilBrown <neilb@suse.de> wrote: >> >> >> mountd can now monitor clients appearing and disappearing in >> /proc/fs/nfsd/clients, and will log these events, in liu of the logging >> of mount/unmount events for NFSv3. >> >> Currently it cannot distinguish between unconfirmed clients (which might >> be transient and totally uninteresting) and confirmed clients. >> >> So add a "status: " line which reports either "confirmed" or >> "unconfirmed", and use fsnotify to report that the info file >> has been modified. >> >> This requires a bit of infrastructure to keep the dentry for the "info" >> file. There is no need to take a counted reference as the dentry must >> remain around until the client is removed. >> >> Signed-off-by: NeilBrown <neilb@suse.de> >> --- >> fs/nfsd/nfs4state.c | 19 +++++++++++++++---- >> fs/nfsd/nfsctl.c | 14 ++++++++------ >> fs/nfsd/nfsd.h | 4 +++- >> fs/nfsd/state.h | 4 ++++ >> 4 files changed, 30 insertions(+), 11 deletions(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index 97447a64bad0..ec1b2dd8fd45 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -43,6 +43,7 @@ >> #include <linux/sunrpc/addr.h> >> #include <linux/jhash.h> >> #include <linux/string_helpers.h> >> +#include <linux/fsnotify.h> >> #include "xdr4.h" >> #include "xdr4cb.h" >> #include "vfs.h" >> @@ -2352,6 +2353,10 @@ static int client_info_show(struct seq_file *m, void *v) >> memcpy(&clid, &clp->cl_clientid, sizeof(clid)); >> seq_printf(m, "clientid: 0x%llx\n", clid); >> seq_printf(m, "address: \"%pISpc\"\n", (struct sockaddr *)&clp->cl_addr); >> + if (test_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags)) >> + seq_puts(m, "status: confirmed\n"); >> + else >> + seq_puts(m, "status: unconfirmed\n"); >> seq_printf(m, "name: "); >> seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len); >> seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion); >> @@ -2702,6 +2707,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, >> int ret; >> struct net *net = SVC_NET(rqstp); >> struct nfsd_net *nn = net_generic(net, nfsd_net_id); >> + struct dentry *dentries[ARRAY_SIZE(client_files)]; >> >> clp = alloc_client(name); >> if (clp == NULL) >> @@ -2721,9 +2727,11 @@ static struct nfs4_client *create_client(struct xdr_netobj name, >> memcpy(&clp->cl_addr, sa, sizeof(struct sockaddr_storage)); >> clp->cl_cb_session = NULL; >> clp->net = net; >> - clp->cl_nfsd_dentry = nfsd_client_mkdir(nn, &clp->cl_nfsdfs, >> - clp->cl_clientid.cl_id - nn->clientid_base, >> - client_files); >> + clp->cl_nfsd_dentry = nfsd_client_mkdir( >> + nn, &clp->cl_nfsdfs, >> + clp->cl_clientid.cl_id - nn->clientid_base, >> + client_files, dentries); >> + clp->cl_nfsd_info_dentry = dentries[0]; >> if (!clp->cl_nfsd_dentry) { >> free_client(clp); >> return NULL; >> @@ -2798,7 +2806,10 @@ move_to_confirmed(struct nfs4_client *clp) >> list_move(&clp->cl_idhash, &nn->conf_id_hashtbl[idhashval]); >> rb_erase(&clp->cl_namenode, &nn->unconf_name_tree); >> add_clp_to_name_tree(clp, &nn->conf_name_tree); >> - set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags); >> + if (!test_and_set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags) && >> + clp->cl_nfsd_dentry && >> + clp->cl_nfsd_info_dentry) >> + fsnotify_dentry(clp->cl_nfsd_info_dentry, FS_MODIFY); > > I hit a "sleep while spin-locked" splat and bisected it to this > commit. fsnotify() can allocate memory with GFP_KERNEL, so it > can't be called while &nn->client_lock is held, unfortunately. Never mind. This was fixed by 934bd07fae7e ("nfsd: move fsnotify on client creation outside spinlock") >> renew_client_locked(clp); >> } >> >> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c >> index ef86ed23af82..94ce1eabd9d1 100644 >> --- a/fs/nfsd/nfsctl.c >> +++ b/fs/nfsd/nfsctl.c >> @@ -1266,7 +1266,8 @@ static void nfsdfs_remove_files(struct dentry *root) >> /* XXX: cut'n'paste from simple_fill_super; figure out if we could share >> * code instead. */ >> static int nfsdfs_create_files(struct dentry *root, >> - const struct tree_descr *files) >> + const struct tree_descr *files, >> + struct dentry **fdentries) >> { >> struct inode *dir = d_inode(root); >> struct inode *inode; >> @@ -1275,8 +1276,6 @@ static int nfsdfs_create_files(struct dentry *root, >> >> inode_lock(dir); >> for (i = 0; files->name && files->name[0]; i++, files++) { >> - if (!files->name) >> - continue; >> dentry = d_alloc_name(root, files->name); >> if (!dentry) >> goto out; >> @@ -1290,6 +1289,8 @@ static int nfsdfs_create_files(struct dentry *root, >> inode->i_private = __get_nfsdfs_client(dir); >> d_add(dentry, inode); >> fsnotify_create(dir, dentry); >> + if (fdentries) >> + fdentries[i] = dentry; >> } >> inode_unlock(dir); >> return 0; >> @@ -1301,8 +1302,9 @@ static int nfsdfs_create_files(struct dentry *root, >> >> /* on success, returns positive number unique to that client. */ >> struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, >> - struct nfsdfs_client *ncl, u32 id, >> - const struct tree_descr *files) >> + struct nfsdfs_client *ncl, u32 id, >> + const struct tree_descr *files, >> + struct dentry **fdentries) >> { >> struct dentry *dentry; >> char name[11]; >> @@ -1313,7 +1315,7 @@ struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, >> dentry = nfsd_mkdir(nn->nfsd_client_dir, ncl, name); >> if (IS_ERR(dentry)) /* XXX: tossing errors? */ >> return NULL; >> - ret = nfsdfs_create_files(dentry, files); >> + ret = nfsdfs_create_files(dentry, files, fdentries); >> if (ret) { >> nfsd_client_rmdir(dentry); >> return NULL; >> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h >> index 8bdc37aa2c2e..9aee72f65330 100644 >> --- a/fs/nfsd/nfsd.h >> +++ b/fs/nfsd/nfsd.h >> @@ -107,7 +107,9 @@ struct nfsdfs_client { >> >> struct nfsdfs_client *get_nfsdfs_client(struct inode *); >> struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, >> - struct nfsdfs_client *ncl, u32 id, const struct tree_descr *); >> + struct nfsdfs_client *ncl, u32 id, >> + const struct tree_descr *, >> + struct dentry **fdentries); >> void nfsd_client_rmdir(struct dentry *dentry); >> >> >> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h >> index 73deea353169..54cab651ac1d 100644 >> --- a/fs/nfsd/state.h >> +++ b/fs/nfsd/state.h >> @@ -371,6 +371,10 @@ struct nfs4_client { >> >> /* debugging info directory under nfsd/clients/ : */ >> struct dentry *cl_nfsd_dentry; >> + /* 'info' file within that directory. Ref is not counted, >> + * but will remain valid iff cl_nfsd_dentry != NULL >> + */ >> + struct dentry *cl_nfsd_info_dentry; >> >> /* for nfs41 callbacks */ >> /* We currently support a single back channel with a single slot */ >> -- >> 2.30.1 >> > > -- > Chuck Lever -- Chuck Lever
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 97447a64bad0..ec1b2dd8fd45 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -43,6 +43,7 @@ #include <linux/sunrpc/addr.h> #include <linux/jhash.h> #include <linux/string_helpers.h> +#include <linux/fsnotify.h> #include "xdr4.h" #include "xdr4cb.h" #include "vfs.h" @@ -2352,6 +2353,10 @@ static int client_info_show(struct seq_file *m, void *v) memcpy(&clid, &clp->cl_clientid, sizeof(clid)); seq_printf(m, "clientid: 0x%llx\n", clid); seq_printf(m, "address: \"%pISpc\"\n", (struct sockaddr *)&clp->cl_addr); + if (test_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags)) + seq_puts(m, "status: confirmed\n"); + else + seq_puts(m, "status: unconfirmed\n"); seq_printf(m, "name: "); seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len); seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion); @@ -2702,6 +2707,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, int ret; struct net *net = SVC_NET(rqstp); struct nfsd_net *nn = net_generic(net, nfsd_net_id); + struct dentry *dentries[ARRAY_SIZE(client_files)]; clp = alloc_client(name); if (clp == NULL) @@ -2721,9 +2727,11 @@ static struct nfs4_client *create_client(struct xdr_netobj name, memcpy(&clp->cl_addr, sa, sizeof(struct sockaddr_storage)); clp->cl_cb_session = NULL; clp->net = net; - clp->cl_nfsd_dentry = nfsd_client_mkdir(nn, &clp->cl_nfsdfs, - clp->cl_clientid.cl_id - nn->clientid_base, - client_files); + clp->cl_nfsd_dentry = nfsd_client_mkdir( + nn, &clp->cl_nfsdfs, + clp->cl_clientid.cl_id - nn->clientid_base, + client_files, dentries); + clp->cl_nfsd_info_dentry = dentries[0]; if (!clp->cl_nfsd_dentry) { free_client(clp); return NULL; @@ -2798,7 +2806,10 @@ move_to_confirmed(struct nfs4_client *clp) list_move(&clp->cl_idhash, &nn->conf_id_hashtbl[idhashval]); rb_erase(&clp->cl_namenode, &nn->unconf_name_tree); add_clp_to_name_tree(clp, &nn->conf_name_tree); - set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags); + if (!test_and_set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags) && + clp->cl_nfsd_dentry && + clp->cl_nfsd_info_dentry) + fsnotify_dentry(clp->cl_nfsd_info_dentry, FS_MODIFY); renew_client_locked(clp); } diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index ef86ed23af82..94ce1eabd9d1 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1266,7 +1266,8 @@ static void nfsdfs_remove_files(struct dentry *root) /* XXX: cut'n'paste from simple_fill_super; figure out if we could share * code instead. */ static int nfsdfs_create_files(struct dentry *root, - const struct tree_descr *files) + const struct tree_descr *files, + struct dentry **fdentries) { struct inode *dir = d_inode(root); struct inode *inode; @@ -1275,8 +1276,6 @@ static int nfsdfs_create_files(struct dentry *root, inode_lock(dir); for (i = 0; files->name && files->name[0]; i++, files++) { - if (!files->name) - continue; dentry = d_alloc_name(root, files->name); if (!dentry) goto out; @@ -1290,6 +1289,8 @@ static int nfsdfs_create_files(struct dentry *root, inode->i_private = __get_nfsdfs_client(dir); d_add(dentry, inode); fsnotify_create(dir, dentry); + if (fdentries) + fdentries[i] = dentry; } inode_unlock(dir); return 0; @@ -1301,8 +1302,9 @@ static int nfsdfs_create_files(struct dentry *root, /* on success, returns positive number unique to that client. */ struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, - struct nfsdfs_client *ncl, u32 id, - const struct tree_descr *files) + struct nfsdfs_client *ncl, u32 id, + const struct tree_descr *files, + struct dentry **fdentries) { struct dentry *dentry; char name[11]; @@ -1313,7 +1315,7 @@ struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, dentry = nfsd_mkdir(nn->nfsd_client_dir, ncl, name); if (IS_ERR(dentry)) /* XXX: tossing errors? */ return NULL; - ret = nfsdfs_create_files(dentry, files); + ret = nfsdfs_create_files(dentry, files, fdentries); if (ret) { nfsd_client_rmdir(dentry); return NULL; diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h index 8bdc37aa2c2e..9aee72f65330 100644 --- a/fs/nfsd/nfsd.h +++ b/fs/nfsd/nfsd.h @@ -107,7 +107,9 @@ struct nfsdfs_client { struct nfsdfs_client *get_nfsdfs_client(struct inode *); struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, - struct nfsdfs_client *ncl, u32 id, const struct tree_descr *); + struct nfsdfs_client *ncl, u32 id, + const struct tree_descr *, + struct dentry **fdentries); void nfsd_client_rmdir(struct dentry *dentry); diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index 73deea353169..54cab651ac1d 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -371,6 +371,10 @@ struct nfs4_client { /* debugging info directory under nfsd/clients/ : */ struct dentry *cl_nfsd_dentry; + /* 'info' file within that directory. Ref is not counted, + * but will remain valid iff cl_nfsd_dentry != NULL + */ + struct dentry *cl_nfsd_info_dentry; /* for nfs41 callbacks */ /* We currently support a single back channel with a single slot */
mountd can now monitor clients appearing and disappearing in /proc/fs/nfsd/clients, and will log these events, in liu of the logging of mount/unmount events for NFSv3. Currently it cannot distinguish between unconfirmed clients (which might be transient and totally uninteresting) and confirmed clients. So add a "status: " line which reports either "confirmed" or "unconfirmed", and use fsnotify to report that the info file has been modified. This requires a bit of infrastructure to keep the dentry for the "info" file. There is no need to take a counted reference as the dentry must remain around until the client is removed. Signed-off-by: NeilBrown <neilb@suse.de> --- fs/nfsd/nfs4state.c | 19 +++++++++++++++---- fs/nfsd/nfsctl.c | 14 ++++++++------ fs/nfsd/nfsd.h | 4 +++- fs/nfsd/state.h | 4 ++++ 4 files changed, 30 insertions(+), 11 deletions(-)