| Message ID | 20210312170943.57461-1-pmenzel@molgen.mpg.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] nfsd: Log error on UMH upcall init failure and debug message on success | expand |
Hi Paul- > On Mar 12, 2021, at 12:09 PM, Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > By default, using `printk()`, Linux logs messages with level warning, > which leaves the user reading > > NFSD: Using UMH upcall client tracking operations. > > wondering what to do about it. Reading `nfsd4_umh_cltrack_init()`, the > message is actually logged on success, so nothing needs to be done, and > it was decided to use the debug level. > > Additionally, Linux now logs an error on init failure. > > NFSD: Failed to init UMH upcall client tracking operations. > > Cc: linux-nfs@vger.kernel.org > Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> > --- > v2: Log error and demote success message to debug-level (forgot `-a` in `git commit --amend`) > v3: Actually sent correct diff > > fs/nfsd/nfs4recover.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c > index 891395c6c7d3..fff89c739033 100644 > --- a/fs/nfsd/nfs4recover.c > +++ b/fs/nfsd/nfs4recover.c > @@ -1863,8 +1863,11 @@ nfsd4_umh_cltrack_init(struct net *net) > > ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL); > kfree(grace_start); > - if (!ret) > - printk("NFSD: Using UMH upcall client tracking operations.\n"); > + if (ret) > + pr_debug("NFSD: Using UMH upcall client tracking operations.\n"); Looking back at 869216075b63 ("nfsd: re-order client tracking method selection"), the idea here seems to be that only one of the "Using yada upcall client" is supposed to fire. If an error occurs here, there is a fallback. No additional error need be reported, IMHO. So the printk() call-sites added by 869216075b63 all need to be adjusted to use pr_info(), I think? > + else > + pr_err("NFSD: Failed to init UMH upcall client tracking operations."); > + > return ret; > } > > -- > 2.30.2 > -- Chuck Lever
On Fri, Mar 12, 2021 at 06:09:44PM +0100, Paul Menzel wrote: > By default, using `printk()`, Linux logs messages with level warning, > which leaves the user reading > > NFSD: Using UMH upcall client tracking operations. > > wondering what to do about it. Reading `nfsd4_umh_cltrack_init()`, the > message is actually logged on success, so nothing needs to be done, and > it was decided to use the debug level. > > Additionally, Linux now logs an error on init failure. > > NFSD: Failed to init UMH upcall client tracking operations. The thing is, it's actually trying a series of different mechanisms (see nfsd4_client_tracking_init) and taking the first one that works. It's more useful to see which one of them ends up being chosen rather than the list of mechanisms that failed. (And those failures are normal if userland is configured to use something lower down in the list.) So, let's just demote to "debug" and leave the logic otherwise unchanged. --b. > > Cc: linux-nfs@vger.kernel.org > Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> > --- > v2: Log error and demote success message to debug-level (forgot `-a` in `git commit --amend`) > v3: Actually sent correct diff > > fs/nfsd/nfs4recover.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c > index 891395c6c7d3..fff89c739033 100644 > --- a/fs/nfsd/nfs4recover.c > +++ b/fs/nfsd/nfs4recover.c > @@ -1863,8 +1863,11 @@ nfsd4_umh_cltrack_init(struct net *net) > > ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL); > kfree(grace_start); > - if (!ret) > - printk("NFSD: Using UMH upcall client tracking operations.\n"); > + if (ret) > + pr_debug("NFSD: Using UMH upcall client tracking operations.\n"); > + else > + pr_err("NFSD: Failed to init UMH upcall client tracking operations."); > + > return ret; > } > > -- > 2.30.2
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c index 891395c6c7d3..fff89c739033 100644 --- a/fs/nfsd/nfs4recover.c +++ b/fs/nfsd/nfs4recover.c @@ -1863,8 +1863,11 @@ nfsd4_umh_cltrack_init(struct net *net) ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL); kfree(grace_start); - if (!ret) - printk("NFSD: Using UMH upcall client tracking operations.\n"); + if (ret) + pr_debug("NFSD: Using UMH upcall client tracking operations.\n"); + else + pr_err("NFSD: Failed to init UMH upcall client tracking operations."); + return ret; }
By default, using `printk()`, Linux logs messages with level warning, which leaves the user reading NFSD: Using UMH upcall client tracking operations. wondering what to do about it. Reading `nfsd4_umh_cltrack_init()`, the message is actually logged on success, so nothing needs to be done, and it was decided to use the debug level. Additionally, Linux now logs an error on init failure. NFSD: Failed to init UMH upcall client tracking operations. Cc: linux-nfs@vger.kernel.org Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> --- v2: Log error and demote success message to debug-level (forgot `-a` in `git commit --amend`) v3: Actually sent correct diff fs/nfsd/nfs4recover.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)