All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/1] Add callback address and state to nfsd4 client info
@ 2021-05-17 20:29 Dave Wysochanski
  2021-05-17 20:29 ` [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client Dave Wysochanski
  0 siblings, 1 reply; 7+ messages in thread
From: Dave Wysochanski @ 2021-05-17 20:29 UTC (permalink / raw)
  To: Bruce Fields, Chuck Lever III; +Cc: linux-nfs

For troubleshooting, it is useful to show the callback address and state
inside the nfsd4 client info.  Note the callback address and state is
also available via trace events, so use a common function and output.

Changes since v1:
- fix indents, run checkpatch (Chuck L)
- create cb_state2str() inside fs/nfsd/trace.c (Bruce F)
- rebase on Chuck v3 nfsd patches and test

Dave Wysochanski (1):
  nfsd4: Expose the callback address and state of each NFS4 client

 fs/nfsd/nfs4state.c |  2 ++
 fs/nfsd/trace.c     | 15 +++++++++++++++
 fs/nfsd/trace.h     |  9 ++-------
 3 files changed, 19 insertions(+), 7 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client
  2021-05-17 20:29 [PATCH v2 0/1] Add callback address and state to nfsd4 client info Dave Wysochanski
@ 2021-05-17 20:29 ` Dave Wysochanski
  2021-05-25 20:58   ` Bruce Fields
  0 siblings, 1 reply; 7+ messages in thread
From: Dave Wysochanski @ 2021-05-17 20:29 UTC (permalink / raw)
  To: Bruce Fields, Chuck Lever III; +Cc: linux-nfs

In addition to the client's address, display the callback channel
state and address in the 'info' file.  Define and use a common
function for this information that can be used by both callback
trace events and the NFS4 client 'info' file.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 fs/nfsd/nfs4state.c |  2 ++
 fs/nfsd/trace.c     | 15 +++++++++++++++
 fs/nfsd/trace.h     |  9 ++-------
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b2573d3ecd3c..f3b8221bb543 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2385,6 +2385,8 @@ static int client_info_show(struct seq_file *m, void *v)
 		seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
 			clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
 	}
+	seq_printf(m, "callback state: %s\n", cb_state2str(clp->cl_cb_state));
+	seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
 	drop_client(clp);
 
 	return 0;
diff --git a/fs/nfsd/trace.c b/fs/nfsd/trace.c
index f008b95ceec2..6291b5d10824 100644
--- a/fs/nfsd/trace.c
+++ b/fs/nfsd/trace.c
@@ -2,3 +2,18 @@
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
+
+const char *cb_state2str(const int state)
+{
+	switch (state) {
+	case NFSD4_CB_UP:
+		return "UP";
+	case NFSD4_CB_UNKNOWN:
+		return "UNKNOWN";
+	case NFSD4_CB_DOWN:
+		return "DOWN";
+	case NFSD4_CB_FAULT:
+		return "FAULT";
+	}
+	return "UNDEFINED";
+}
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 10cc3aaf1089..8908d48b2aa6 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -876,12 +876,7 @@
 	TP_printk("client %08x:%08x", __entry->cl_boot, __entry->cl_id)
 )
 
-#define show_cb_state(val)						\
-	__print_symbolic(val,						\
-		{ NFSD4_CB_UP,		"UP" },				\
-		{ NFSD4_CB_UNKNOWN,	"UNKNOWN" },			\
-		{ NFSD4_CB_DOWN,	"DOWN" },			\
-		{ NFSD4_CB_FAULT,	"FAULT"})
+const char *cb_state2str(const int state);
 
 DECLARE_EVENT_CLASS(nfsd_cb_class,
 	TP_PROTO(const struct nfs4_client *clp),
@@ -901,7 +896,7 @@
 	),
 	TP_printk("addr=%pISpc client %08x:%08x state=%s",
 		__entry->addr, __entry->cl_boot, __entry->cl_id,
-		show_cb_state(__entry->state))
+		cb_state2str(__entry->state))
 );
 
 #define DEFINE_NFSD_CB_EVENT(name)			\
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client
  2021-05-17 20:29 ` [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client Dave Wysochanski
@ 2021-05-25 20:58   ` Bruce Fields
  2021-05-25 21:48     ` Chuck Lever III
  2021-05-26  1:32     ` Trond Myklebust
  0 siblings, 2 replies; 7+ messages in thread
From: Bruce Fields @ 2021-05-25 20:58 UTC (permalink / raw)
  To: Dave Wysochanski; +Cc: Chuck Lever III, linux-nfs

When I run trace-cmd report I get output like:

  [nfsd:nfsd_cb_state] function cb_state2str not defined
  [nfsd:nfsd_cb_shutdown] function cb_state2str not defined
  [nfsd:nfsd_cb_probe] function cb_state2str not defined
  [nfsd:nfsd_cb_lost] function cb_state2str not defined

I don't know how this is supposed to work.  Is it OK for tracepoint definitions
to reference kernel functions if they're defined in the right way somehow?  If
not, I don't know what the solution would be for sharing this--define a macro
that expands to the array literal and use that in both places?  Or maybe just
live with the the redundancy.

--b.

On Mon, May 17, 2021 at 04:29:45PM -0400, Dave Wysochanski wrote:
> In addition to the client's address, display the callback channel
> state and address in the 'info' file.  Define and use a common
> function for this information that can be used by both callback
> trace events and the NFS4 client 'info' file.
> 
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> ---
>  fs/nfsd/nfs4state.c |  2 ++
>  fs/nfsd/trace.c     | 15 +++++++++++++++
>  fs/nfsd/trace.h     |  9 ++-------
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b2573d3ecd3c..f3b8221bb543 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2385,6 +2385,8 @@ static int client_info_show(struct seq_file *m, void *v)
>  		seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
>  			clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
>  	}
> +	seq_printf(m, "callback state: %s\n", cb_state2str(clp->cl_cb_state));
> +	seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
>  	drop_client(clp);
>  
>  	return 0;
> diff --git a/fs/nfsd/trace.c b/fs/nfsd/trace.c
> index f008b95ceec2..6291b5d10824 100644
> --- a/fs/nfsd/trace.c
> +++ b/fs/nfsd/trace.c
> @@ -2,3 +2,18 @@
>  
>  #define CREATE_TRACE_POINTS
>  #include "trace.h"
> +
> +const char *cb_state2str(const int state)
> +{
> +	switch (state) {
> +	case NFSD4_CB_UP:
> +		return "UP";
> +	case NFSD4_CB_UNKNOWN:
> +		return "UNKNOWN";
> +	case NFSD4_CB_DOWN:
> +		return "DOWN";
> +	case NFSD4_CB_FAULT:
> +		return "FAULT";
> +	}
> +	return "UNDEFINED";
> +}
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 10cc3aaf1089..8908d48b2aa6 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -876,12 +876,7 @@
>  	TP_printk("client %08x:%08x", __entry->cl_boot, __entry->cl_id)
>  )
>  
> -#define show_cb_state(val)						\
> -	__print_symbolic(val,						\
> -		{ NFSD4_CB_UP,		"UP" },				\
> -		{ NFSD4_CB_UNKNOWN,	"UNKNOWN" },			\
> -		{ NFSD4_CB_DOWN,	"DOWN" },			\
> -		{ NFSD4_CB_FAULT,	"FAULT"})
> +const char *cb_state2str(const int state);
>  
>  DECLARE_EVENT_CLASS(nfsd_cb_class,
>  	TP_PROTO(const struct nfs4_client *clp),
> @@ -901,7 +896,7 @@
>  	),
>  	TP_printk("addr=%pISpc client %08x:%08x state=%s",
>  		__entry->addr, __entry->cl_boot, __entry->cl_id,
> -		show_cb_state(__entry->state))
> +		cb_state2str(__entry->state))
>  );
>  
>  #define DEFINE_NFSD_CB_EVENT(name)			\
> -- 
> 1.8.3.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client
  2021-05-25 20:58   ` Bruce Fields
@ 2021-05-25 21:48     ` Chuck Lever III
  2021-05-26 18:30       ` Bruce Fields
  2021-05-26  1:32     ` Trond Myklebust
  1 sibling, 1 reply; 7+ messages in thread
From: Chuck Lever III @ 2021-05-25 21:48 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Dave Wysochanski, Linux NFS Mailing List



> On May 25, 2021, at 4:58 PM, Bruce Fields <bfields@fieldses.org> wrote:
> 
> When I run trace-cmd report I get output like:
> 
>  [nfsd:nfsd_cb_state] function cb_state2str not defined
>  [nfsd:nfsd_cb_shutdown] function cb_state2str not defined
>  [nfsd:nfsd_cb_probe] function cb_state2str not defined
>  [nfsd:nfsd_cb_lost] function cb_state2str not defined
> 
> I don't know how this is supposed to work.  Is it OK for tracepoint definitions
> to reference kernel functions if they're defined in the right way somehow?  If
> not, I don't know what the solution would be for sharing this--define a macro
> that expands to the array literal and use that in both places?  Or maybe just
> live with the the redundancy.

Living with the redundancy is OK with me.


> --b.
> 
> On Mon, May 17, 2021 at 04:29:45PM -0400, Dave Wysochanski wrote:
>> In addition to the client's address, display the callback channel
>> state and address in the 'info' file.  Define and use a common
>> function for this information that can be used by both callback
>> trace events and the NFS4 client 'info' file.
>> 
>> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
>> ---
>> fs/nfsd/nfs4state.c |  2 ++
>> fs/nfsd/trace.c     | 15 +++++++++++++++
>> fs/nfsd/trace.h     |  9 ++-------
>> 3 files changed, 19 insertions(+), 7 deletions(-)
>> 
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index b2573d3ecd3c..f3b8221bb543 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -2385,6 +2385,8 @@ static int client_info_show(struct seq_file *m, void *v)
>> 		seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
>> 			clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
>> 	}
>> +	seq_printf(m, "callback state: %s\n", cb_state2str(clp->cl_cb_state));
>> +	seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
>> 	drop_client(clp);
>> 
>> 	return 0;
>> diff --git a/fs/nfsd/trace.c b/fs/nfsd/trace.c
>> index f008b95ceec2..6291b5d10824 100644
>> --- a/fs/nfsd/trace.c
>> +++ b/fs/nfsd/trace.c
>> @@ -2,3 +2,18 @@
>> 
>> #define CREATE_TRACE_POINTS
>> #include "trace.h"
>> +
>> +const char *cb_state2str(const int state)
>> +{
>> +	switch (state) {
>> +	case NFSD4_CB_UP:
>> +		return "UP";
>> +	case NFSD4_CB_UNKNOWN:
>> +		return "UNKNOWN";
>> +	case NFSD4_CB_DOWN:
>> +		return "DOWN";
>> +	case NFSD4_CB_FAULT:
>> +		return "FAULT";
>> +	}
>> +	return "UNDEFINED";
>> +}
>> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
>> index 10cc3aaf1089..8908d48b2aa6 100644
>> --- a/fs/nfsd/trace.h
>> +++ b/fs/nfsd/trace.h
>> @@ -876,12 +876,7 @@
>> 	TP_printk("client %08x:%08x", __entry->cl_boot, __entry->cl_id)
>> )
>> 
>> -#define show_cb_state(val)						\
>> -	__print_symbolic(val,						\
>> -		{ NFSD4_CB_UP,		"UP" },				\
>> -		{ NFSD4_CB_UNKNOWN,	"UNKNOWN" },			\
>> -		{ NFSD4_CB_DOWN,	"DOWN" },			\
>> -		{ NFSD4_CB_FAULT,	"FAULT"})
>> +const char *cb_state2str(const int state);
>> 
>> DECLARE_EVENT_CLASS(nfsd_cb_class,
>> 	TP_PROTO(const struct nfs4_client *clp),
>> @@ -901,7 +896,7 @@
>> 	),
>> 	TP_printk("addr=%pISpc client %08x:%08x state=%s",
>> 		__entry->addr, __entry->cl_boot, __entry->cl_id,
>> -		show_cb_state(__entry->state))
>> +		cb_state2str(__entry->state))
>> );
>> 
>> #define DEFINE_NFSD_CB_EVENT(name)			\
>> -- 
>> 1.8.3.1

--
Chuck Lever




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client
  2021-05-25 20:58   ` Bruce Fields
  2021-05-25 21:48     ` Chuck Lever III
@ 2021-05-26  1:32     ` Trond Myklebust
  1 sibling, 0 replies; 7+ messages in thread
From: Trond Myklebust @ 2021-05-26  1:32 UTC (permalink / raw)
  To: bfields, dwysocha; +Cc: linux-nfs, chuck.lever

On Tue, 2021-05-25 at 16:58 -0400, Bruce Fields wrote:
> When I run trace-cmd report I get output like:
> 
>   [nfsd:nfsd_cb_state] function cb_state2str not defined
>   [nfsd:nfsd_cb_shutdown] function cb_state2str not defined
>   [nfsd:nfsd_cb_probe] function cb_state2str not defined
>   [nfsd:nfsd_cb_lost] function cb_state2str not defined
> 
> I don't know how this is supposed to work.  Is it OK for tracepoint
> definitions
> to reference kernel functions if they're defined in the right way
> somehow?  If
> not, I don't know what the solution would be for sharing this--define
> a macro
> that expands to the array literal and use that in both places?  Or
> maybe just
> live with the the redundancy.
> 

You need to store the string in the TP_fast_assign() section if you
want to be able to display it. Otherwise 'perf' tries (and fails) to
find the cb_state2str function so it can invoke it.

> --b.
> 
> On Mon, May 17, 2021 at 04:29:45PM -0400, Dave Wysochanski wrote:
> > In addition to the client's address, display the callback channel
> > state and address in the 'info' file.  Define and use a common
> > function for this information that can be used by both callback
> > trace events and the NFS4 client 'info' file.
> > 
> > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> > ---
> >  fs/nfsd/nfs4state.c |  2 ++
> >  fs/nfsd/trace.c     | 15 +++++++++++++++
> >  fs/nfsd/trace.h     |  9 ++-------
> >  3 files changed, 19 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index b2573d3ecd3c..f3b8221bb543 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -2385,6 +2385,8 @@ static int client_info_show(struct seq_file
> > *m, void *v)
> >                 seq_printf(m, "\nImplementation time: [%lld,
> > %ld]\n",
> >                         clp->cl_nii_time.tv_sec, clp-
> > >cl_nii_time.tv_nsec);
> >         }
> > +       seq_printf(m, "callback state: %s\n", cb_state2str(clp-
> > >cl_cb_state));
> > +       seq_printf(m, "callback address: %pISpc\n", &clp-
> > >cl_cb_conn.cb_addr);
> >         drop_client(clp);
> >  
> >         return 0;
> > diff --git a/fs/nfsd/trace.c b/fs/nfsd/trace.c
> > index f008b95ceec2..6291b5d10824 100644
> > --- a/fs/nfsd/trace.c
> > +++ b/fs/nfsd/trace.c
> > @@ -2,3 +2,18 @@
> >  
> >  #define CREATE_TRACE_POINTS
> >  #include "trace.h"
> > +
> > +const char *cb_state2str(const int state)
> > +{
> > +       switch (state) {
> > +       case NFSD4_CB_UP:
> > +               return "UP";
> > +       case NFSD4_CB_UNKNOWN:
> > +               return "UNKNOWN";
> > +       case NFSD4_CB_DOWN:
> > +               return "DOWN";
> > +       case NFSD4_CB_FAULT:
> > +               return "FAULT";
> > +       }
> > +       return "UNDEFINED";
> > +}
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index 10cc3aaf1089..8908d48b2aa6 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -876,12 +876,7 @@
> >         TP_printk("client %08x:%08x", __entry->cl_boot, __entry-
> > >cl_id)
> >  )
> >  
> > -#define
> > show_cb_state(val)                                             \
> > -
> >        __print_symbolic(val,                                        
> >    \
> > -               { NFSD4_CB_UP,          "UP"
> > },                         \
> > -               { NFSD4_CB_UNKNOWN,     "UNKNOWN"
> > },                    \
> > -               { NFSD4_CB_DOWN,        "DOWN"
> > },                       \
> > -               { NFSD4_CB_FAULT,       "FAULT"})
> > +const char *cb_state2str(const int state);
> >  
> >  DECLARE_EVENT_CLASS(nfsd_cb_class,
> >         TP_PROTO(const struct nfs4_client *clp),
> > @@ -901,7 +896,7 @@
> >         ),
> >         TP_printk("addr=%pISpc client %08x:%08x state=%s",
> >                 __entry->addr, __entry->cl_boot, __entry->cl_id,
> > -               show_cb_state(__entry->state))
> > +               cb_state2str(__entry->state))
> >  );
> >  
> >  #define DEFINE_NFSD_CB_EVENT(name)                     \
> > -- 
> > 1.8.3.1

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client
  2021-05-25 21:48     ` Chuck Lever III
@ 2021-05-26 18:30       ` Bruce Fields
  2021-05-26 19:13         ` David Wysochanski
  0 siblings, 1 reply; 7+ messages in thread
From: Bruce Fields @ 2021-05-26 18:30 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Dave Wysochanski, Linux NFS Mailing List

On Tue, May 25, 2021 at 09:48:38PM +0000, Chuck Lever III wrote:
> 
> 
> > On May 25, 2021, at 4:58 PM, Bruce Fields <bfields@fieldses.org> wrote:
> > 
> > When I run trace-cmd report I get output like:
> > 
> >  [nfsd:nfsd_cb_state] function cb_state2str not defined
> >  [nfsd:nfsd_cb_shutdown] function cb_state2str not defined
> >  [nfsd:nfsd_cb_probe] function cb_state2str not defined
> >  [nfsd:nfsd_cb_lost] function cb_state2str not defined
> > 
> > I don't know how this is supposed to work.  Is it OK for tracepoint definitions
> > to reference kernel functions if they're defined in the right way somehow?  If
> > not, I don't know what the solution would be for sharing this--define a macro
> > that expands to the array literal and use that in both places?  Or maybe just
> > live with the the redundancy.
> 
> Living with the redundancy is OK with me.

OK, I'll revert back to Dave's first version of this patch.

--b.

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 49c052243b5c..89a7cada334d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2357,6 +2357,21 @@ static void seq_quote_mem(struct seq_file *m, char *data, int len)
 	seq_printf(m, "\"");
 }
 
+static const char *cb_state_str(int state)
+{
+	switch (state) {
+		case NFSD4_CB_UP:
+			return "UP";
+		case NFSD4_CB_UNKNOWN:
+			return "UNKNOWN";
+		case NFSD4_CB_DOWN:
+			return "DOWN";
+		case NFSD4_CB_FAULT:
+			return "FAULT";
+	}
+	return "UNDEFINED";
+}
+
 static int client_info_show(struct seq_file *m, void *v)
 {
 	struct inode *inode = m->private;
@@ -2385,6 +2400,8 @@ static int client_info_show(struct seq_file *m, void *v)
 		seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
 			clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
 	}
+	seq_printf(m, "callback state: %s\n", cb_state_str(clp->cl_cb_state));
+	seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
 	drop_client(clp);
 
 	return 0;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client
  2021-05-26 18:30       ` Bruce Fields
@ 2021-05-26 19:13         ` David Wysochanski
  0 siblings, 0 replies; 7+ messages in thread
From: David Wysochanski @ 2021-05-26 19:13 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Chuck Lever III, Linux NFS Mailing List

On Wed, May 26, 2021 at 2:30 PM Bruce Fields <bfields@fieldses.org> wrote:
>
> On Tue, May 25, 2021 at 09:48:38PM +0000, Chuck Lever III wrote:
> >
> >
> > > On May 25, 2021, at 4:58 PM, Bruce Fields <bfields@fieldses.org> wrote:
> > >
> > > When I run trace-cmd report I get output like:
> > >
> > >  [nfsd:nfsd_cb_state] function cb_state2str not defined
> > >  [nfsd:nfsd_cb_shutdown] function cb_state2str not defined
> > >  [nfsd:nfsd_cb_probe] function cb_state2str not defined
> > >  [nfsd:nfsd_cb_lost] function cb_state2str not defined
> > >
> > > I don't know how this is supposed to work.  Is it OK for tracepoint definitions
> > > to reference kernel functions if they're defined in the right way somehow?  If
> > > not, I don't know what the solution would be for sharing this--define a macro
> > > that expands to the array literal and use that in both places?  Or maybe just
> > > live with the the redundancy.
> >
> > Living with the redundancy is OK with me.
>
> OK, I'll revert back to Dave's first version of this patch.
>

I'm ok with the above.

If reuse was desired we could do something like this, but it's a bit
ugly / convoluted:

diff --git a/fs/nfsd/trace.c b/fs/nfsd/trace.c
index f008b95ceec2..54c31b5f42d4 100644
--- a/fs/nfsd/trace.c
+++ b/fs/nfsd/trace.c
@@ -2,3 +2,16 @@

 #define CREATE_TRACE_POINTS
 #include "trace.h"
+
+const char *cb_state2str(const int state)
+{
+       int i;
+       const struct trace_print_flags state_array[] = { CB_STATE_ARRAY,
+                                                        { -1, NULL } };
+
+       for (i = 0; state_array[i].name; i++)
+               if (state == state_array[i].mask)
+                       return state_array[i].name;
+
+       return "UNDEFINED";
+}
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 10cc3aaf1089..f21841f4ae3b 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -876,12 +876,12 @@ TRACE_EVENT(nfsd_cb_nodelegs,
        TP_printk("client %08x:%08x", __entry->cl_boot, __entry->cl_id)
 )

-#define show_cb_state(val)                                             \
-       __print_symbolic(val,                                           \
-               { NFSD4_CB_UP,          "UP" },                         \
-               { NFSD4_CB_UNKNOWN,     "UNKNOWN" },                    \
-               { NFSD4_CB_DOWN,        "DOWN" },                       \
-               { NFSD4_CB_FAULT,       "FAULT"})
+#define CB_STATE_ARRAY \
+       { NFSD4_CB_UP,          "UP" },                 \
+       { NFSD4_CB_UNKNOWN,     "UNKNOWN" },            \
+       { NFSD4_CB_DOWN,        "DOWN" },               \
+       { NFSD4_CB_FAULT,       "FAULT"}
+extern const char *cb_state2str(const int state);

 DECLARE_EVENT_CLASS(nfsd_cb_class,
        TP_PROTO(const struct nfs4_client *clp),
@@ -901,7 +901,7 @@ DECLARE_EVENT_CLASS(nfsd_cb_class,
        ),
        TP_printk("addr=%pISpc client %08x:%08x state=%s",
                __entry->addr, __entry->cl_boot, __entry->cl_id,
-               show_cb_state(__entry->state))
+               __print_symbolic(__entry->state, CB_STATE_ARRAY))
 );

 #define DEFINE_NFSD_CB_EVENT(name)                     \


> --b.
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 49c052243b5c..89a7cada334d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2357,6 +2357,21 @@ static void seq_quote_mem(struct seq_file *m, char *data, int len)
>         seq_printf(m, "\"");
>  }
>
> +static const char *cb_state_str(int state)
> +{
> +       switch (state) {
> +               case NFSD4_CB_UP:
> +                       return "UP";
> +               case NFSD4_CB_UNKNOWN:
> +                       return "UNKNOWN";
> +               case NFSD4_CB_DOWN:
> +                       return "DOWN";
> +               case NFSD4_CB_FAULT:
> +                       return "FAULT";
> +       }
> +       return "UNDEFINED";
> +}
> +
>  static int client_info_show(struct seq_file *m, void *v)
>  {
>         struct inode *inode = m->private;
> @@ -2385,6 +2400,8 @@ static int client_info_show(struct seq_file *m, void *v)
>                 seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
>                         clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
>         }
> +       seq_printf(m, "callback state: %s\n", cb_state_str(clp->cl_cb_state));
> +       seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
>         drop_client(clp);
>
>         return 0;
> --
> 1.8.3.1
>


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-05-26 19:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 20:29 [PATCH v2 0/1] Add callback address and state to nfsd4 client info Dave Wysochanski
2021-05-17 20:29 ` [PATCH v2 1/1] nfsd4: Expose the callback address and state of each NFS4 client Dave Wysochanski
2021-05-25 20:58   ` Bruce Fields
2021-05-25 21:48     ` Chuck Lever III
2021-05-26 18:30       ` Bruce Fields
2021-05-26 19:13         ` David Wysochanski
2021-05-26  1:32     ` Trond Myklebust

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.