* [PATCH] nfsd: Demote UMH upcall init message from warning- to info-level
@ 2021-03-12 11:28 Paul Menzel
2021-03-12 11:33 ` Paul Menzel
0 siblings, 1 reply; 9+ messages in thread
From: Paul Menzel @ 2021-03-12 11:28 UTC (permalink / raw)
To: J . Bruce Fields, Chuck Lever; +Cc: Paul Menzel, linux-nfs
By default, using `printk()`, Linux logs messages with level warning,
which leaves the user seeing
NFSD: Using UMH upcall client tracking operations.
what to do about it. Reading `nfsd4_umh_cltrack_init()`, the message is
actually logged on success, so nothing needs to be done, and the info
level should be used.
Cc: linux-nfs@vger.kernel.org
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
fs/nfsd/nfs4recover.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 891395c6c7d3..db66c45a6b97 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -1864,7 +1864,7 @@ 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");
+ pr_info("NFSD: Using UMH upcall client tracking operations.\n");
return ret;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] nfsd: Demote UMH upcall init message from warning- to info-level
2021-03-12 11:28 [PATCH] nfsd: Demote UMH upcall init message from warning- to info-level Paul Menzel
@ 2021-03-12 11:33 ` Paul Menzel
2021-03-12 15:32 ` J . Bruce Fields
0 siblings, 1 reply; 9+ messages in thread
From: Paul Menzel @ 2021-03-12 11:33 UTC (permalink / raw)
To: J . Bruce Fields, Chuck Lever; +Cc: linux-nfs
Dear Linux folks,
Am 12.03.21 um 12:28 schrieb Paul Menzel:
> By default, using `printk()`, Linux logs messages with level warning,
> which leaves the user seeing
>
> NFSD: Using UMH upcall client tracking operations.
>
> what to do about it. Reading `nfsd4_umh_cltrack_init()`, the message is
> actually logged on success, so nothing needs to be done, and the info
> level should be used.
>
> Cc: linux-nfs@vger.kernel.org
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
> fs/nfsd/nfs4recover.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 891395c6c7d3..db66c45a6b97 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -1864,7 +1864,7 @@ 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");
> + pr_info("NFSD: Using UMH upcall client tracking operations.\n");
> return ret;
> }
A debug-level message could also be used, or the line totally removed,
and the condition be changed to print an error in case of failure. I am
wondering about the benefit for the user reading through the logs. Maybe
the log was only there, because UMH upcall client tracking operations
were something new?
Kind regards,
Paul
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] nfsd: Demote UMH upcall init message from warning- to info-level
2021-03-12 11:33 ` Paul Menzel
@ 2021-03-12 15:32 ` J . Bruce Fields
2021-03-12 17:06 ` [PATCH v2] nfsd: Log error on UMH upcall init failure and debug message on success Paul Menzel
0 siblings, 1 reply; 9+ messages in thread
From: J . Bruce Fields @ 2021-03-12 15:32 UTC (permalink / raw)
To: Paul Menzel; +Cc: Chuck Lever, linux-nfs
On Fri, Mar 12, 2021 at 12:33:00PM +0100, Paul Menzel wrote:
> Dear Linux folks,
>
>
> Am 12.03.21 um 12:28 schrieb Paul Menzel:
> >By default, using `printk()`, Linux logs messages with level warning,
> >which leaves the user seeing
> >
> > NFSD: Using UMH upcall client tracking operations.
> >
> >what to do about it. Reading `nfsd4_umh_cltrack_init()`, the message is
> >actually logged on success, so nothing needs to be done, and the info
> >level should be used.
> >
> >Cc: linux-nfs@vger.kernel.org
> >Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> >---
> > fs/nfsd/nfs4recover.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> >index 891395c6c7d3..db66c45a6b97 100644
> >--- a/fs/nfsd/nfs4recover.c
> >+++ b/fs/nfsd/nfs4recover.c
> >@@ -1864,7 +1864,7 @@ 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");
> >+ pr_info("NFSD: Using UMH upcall client tracking operations.\n");
> > return ret;
> > }
>
> A debug-level message could also be used, or the line totally
> removed, and the condition be changed to print an error in case of
> failure. I am wondering about the benefit for the user reading
> through the logs. Maybe the log was only there, because UMH upcall
> client tracking operations were something new?
Could be.
I think debug-level would be fine.
--b.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] nfsd: Log error on UMH upcall init failure and debug message on success
2021-03-12 15:32 ` J . Bruce Fields
@ 2021-03-12 17:06 ` Paul Menzel
2021-03-12 17:09 ` [PATCH v3] " Paul Menzel
0 siblings, 1 reply; 9+ messages in thread
From: Paul Menzel @ 2021-03-12 17:06 UTC (permalink / raw)
To: J . Bruce Fields, Chuck Lever; +Cc: Paul Menzel, linux-nfs
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>
---
fs/nfsd/nfs4recover.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 891395c6c7d3..db66c45a6b97 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -1864,7 +1864,7 @@ 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");
+ pr_info("NFSD: Using UMH upcall client tracking operations.\n");
return ret;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3] nfsd: Log error on UMH upcall init failure and debug message on success
2021-03-12 17:06 ` [PATCH v2] nfsd: Log error on UMH upcall init failure and debug message on success Paul Menzel
@ 2021-03-12 17:09 ` Paul Menzel
2021-03-12 18:59 ` Chuck Lever III
2021-03-12 19:06 ` J . Bruce Fields
0 siblings, 2 replies; 9+ messages in thread
From: Paul Menzel @ 2021-03-12 17:09 UTC (permalink / raw)
To: J . Bruce Fields, Chuck Lever; +Cc: Paul Menzel, linux-nfs
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");
+ else
+ pr_err("NFSD: Failed to init UMH upcall client tracking operations.");
+
return ret;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] nfsd: Log error on UMH upcall init failure and debug message on success
2021-03-12 17:09 ` [PATCH v3] " Paul Menzel
@ 2021-03-12 18:59 ` Chuck Lever III
2021-03-12 19:06 ` J . Bruce Fields
1 sibling, 0 replies; 9+ messages in thread
From: Chuck Lever III @ 2021-03-12 18:59 UTC (permalink / raw)
To: Paul Menzel; +Cc: Bruce Fields, Linux NFS Mailing List
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] nfsd: Log error on UMH upcall init failure and debug message on success
2021-03-12 17:09 ` [PATCH v3] " Paul Menzel
2021-03-12 18:59 ` Chuck Lever III
@ 2021-03-12 19:06 ` J . Bruce Fields
2021-03-12 21:03 ` [PATCH v4] nfsd: Log client tracking type log message as info instead of warning Paul Menzel
1 sibling, 1 reply; 9+ messages in thread
From: J . Bruce Fields @ 2021-03-12 19:06 UTC (permalink / raw)
To: Paul Menzel; +Cc: Chuck Lever, linux-nfs
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4] nfsd: Log client tracking type log message as info instead of warning
2021-03-12 19:06 ` J . Bruce Fields
@ 2021-03-12 21:03 ` Paul Menzel
2021-03-15 14:29 ` Chuck Lever III
0 siblings, 1 reply; 9+ messages in thread
From: Paul Menzel @ 2021-03-12 21:03 UTC (permalink / raw)
To: Bruce Fields, Chuck Lever III; +Cc: Paul Menzel, linux-nfs
`printk()`, by default, uses the log level warning, which leaves the
user reading
NFSD: Using UMH upcall client tracking operations.
wondering what to do about it (`dmesg --level=warn`).
Several client tracking methods are tried, and expected to fail. That’s
why a message is printed only on success. It might be interesting for
users to know the chosen method, so use info-level instead of
debug-level.
Cc: linux-nfs@vger.kernel.org
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
v4: Remove error message, and use info-level, as several client tracking
methods are tried.
fs/nfsd/nfs4recover.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 891395c6c7d3..6fedc49726bf 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -626,7 +626,7 @@ nfsd4_legacy_tracking_init(struct net *net)
status = nfsd4_load_reboot_recovery_data(net);
if (status)
goto err;
- printk("NFSD: Using legacy client tracking operations.\n");
+ pr_info("NFSD: Using legacy client tracking operations.\n");
return 0;
err:
@@ -1028,7 +1028,7 @@ nfsd4_init_cld_pipe(struct net *net)
status = __nfsd4_init_cld_pipe(net);
if (!status)
- printk("NFSD: Using old nfsdcld client tracking operations.\n");
+ pr_info("NFSD: Using old nfsdcld client tracking operations.\n");
return status;
}
@@ -1605,7 +1605,7 @@ nfsd4_cld_tracking_init(struct net *net)
nfs4_release_reclaim(nn);
goto err_remove;
} else
- printk("NFSD: Using nfsdcld client tracking operations.\n");
+ pr_info("NFSD: Using nfsdcld client tracking operations.\n");
return 0;
err_remove:
@@ -1864,7 +1864,7 @@ 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");
+ pr_info("NFSD: Using UMH upcall client tracking operations.\n");
return ret;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4] nfsd: Log client tracking type log message as info instead of warning
2021-03-12 21:03 ` [PATCH v4] nfsd: Log client tracking type log message as info instead of warning Paul Menzel
@ 2021-03-15 14:29 ` Chuck Lever III
0 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever III @ 2021-03-15 14:29 UTC (permalink / raw)
To: Paul Menzel; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 12, 2021, at 4:03 PM, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> `printk()`, by default, uses the log level warning, which leaves the
> user reading
>
> NFSD: Using UMH upcall client tracking operations.
>
> wondering what to do about it (`dmesg --level=warn`).
>
> Several client tracking methods are tried, and expected to fail. That’s
> why a message is printed only on success. It might be interesting for
> users to know the chosen method, so use info-level instead of
> debug-level.
>
> Cc: linux-nfs@vger.kernel.org
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Hi Paul, I've included your patch in the for-next topic branch at
git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
> ---
> v4: Remove error message, and use info-level, as several client tracking
> methods are tried.
>
> fs/nfsd/nfs4recover.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 891395c6c7d3..6fedc49726bf 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -626,7 +626,7 @@ nfsd4_legacy_tracking_init(struct net *net)
> status = nfsd4_load_reboot_recovery_data(net);
> if (status)
> goto err;
> - printk("NFSD: Using legacy client tracking operations.\n");
> + pr_info("NFSD: Using legacy client tracking operations.\n");
> return 0;
>
> err:
> @@ -1028,7 +1028,7 @@ nfsd4_init_cld_pipe(struct net *net)
>
> status = __nfsd4_init_cld_pipe(net);
> if (!status)
> - printk("NFSD: Using old nfsdcld client tracking operations.\n");
> + pr_info("NFSD: Using old nfsdcld client tracking operations.\n");
> return status;
> }
>
> @@ -1605,7 +1605,7 @@ nfsd4_cld_tracking_init(struct net *net)
> nfs4_release_reclaim(nn);
> goto err_remove;
> } else
> - printk("NFSD: Using nfsdcld client tracking operations.\n");
> + pr_info("NFSD: Using nfsdcld client tracking operations.\n");
> return 0;
>
> err_remove:
> @@ -1864,7 +1864,7 @@ 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");
> + pr_info("NFSD: Using UMH upcall client tracking operations.\n");
> return ret;
> }
>
> --
> 2.30.2
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-15 14:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 11:28 [PATCH] nfsd: Demote UMH upcall init message from warning- to info-level Paul Menzel
2021-03-12 11:33 ` Paul Menzel
2021-03-12 15:32 ` J . Bruce Fields
2021-03-12 17:06 ` [PATCH v2] nfsd: Log error on UMH upcall init failure and debug message on success Paul Menzel
2021-03-12 17:09 ` [PATCH v3] " Paul Menzel
2021-03-12 18:59 ` Chuck Lever III
2021-03-12 19:06 ` J . Bruce Fields
2021-03-12 21:03 ` [PATCH v4] nfsd: Log client tracking type log message as info instead of warning Paul Menzel
2021-03-15 14:29 ` Chuck Lever III
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.