On Wed, May 26, 2021 at 2:30 PM Bruce Fields <bfields@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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
>
