[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4838ad92a359a10944487bbcb74690a51dd0a2f8.camel@redhat.com>
Date: Tue, 27 Feb 2024 11:04:50 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Steven Rostedt <rostedt@...dmis.org>, Tobias Waldekranz
<tobias@...dekranz.com>
Cc: davem@...emloft.net, kuba@...nel.org, roopa@...dia.com,
razor@...ckwall.org, bridge@...ts.linux.dev, netdev@...r.kernel.org,
jiri@...nulli.us, ivecera@...hat.com, mhiramat@...nel.org,
linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH v3 net-next 4/4] net: switchdev: Add tracepoints
On Fri, 2024-02-23 at 10:38 -0500, Steven Rostedt wrote:
> On Fri, 23 Feb 2024 12:44:53 +0100
> Tobias Waldekranz <tobias@...dekranz.com> wrote:
>
> > Add a basic set of tracepoints:
> >
> > - switchdev_defer: Fires whenever an operation is enqueued to the
> > switchdev workqueue for deferred delivery.
> >
> > - switchdev_call_{atomic,blocking}: Fires whenever a notification is
> > sent to the corresponding switchdev notifier chain.
> >
> > - switchdev_call_replay: Fires whenever a notification is sent to a
> > specific driver's notifier block, in response to a replay request.
> >
> > Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
> > ---
> > include/trace/events/switchdev.h | 74 ++++++++++++++++++++++++++++++++
> > net/switchdev/switchdev.c | 71 +++++++++++++++++++++++++-----
> > 2 files changed, 135 insertions(+), 10 deletions(-)
> > create mode 100644 include/trace/events/switchdev.h
> >
> > diff --git a/include/trace/events/switchdev.h b/include/trace/events/switchdev.h
> > new file mode 100644
> > index 000000000000..dcaf6870d017
> > --- /dev/null
> > +++ b/include/trace/events/switchdev.h
> > @@ -0,0 +1,74 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#undef TRACE_SYSTEM
> > +#define TRACE_SYSTEM switchdev
> > +
> > +#if !defined(_TRACE_SWITCHDEV_H) || defined(TRACE_HEADER_MULTI_READ)
> > +#define _TRACE_SWITCHDEV_H
> > +
> > +#include <linux/tracepoint.h>
> > +#include <net/switchdev.h>
> > +
> > +#define SWITCHDEV_TRACE_MSG_MAX 128
>
> 128 bytes is awfully big to waste on the ring buffer. What's the average
> size of a string?
>
> > +
> > +DECLARE_EVENT_CLASS(switchdev_call,
> > + TP_PROTO(unsigned long val,
> > + const struct switchdev_notifier_info *info,
> > + int err),
> > +
> > + TP_ARGS(val, info, err),
> > +
> > + TP_STRUCT__entry(
> > + __field(unsigned long, val)
> > + __string(dev, info->dev ? netdev_name(info->dev) : "(null)")
> > + __field(const struct switchdev_notifier_info *, info)
> > + __field(int, err)
> > + __array(char, msg, SWITCHDEV_TRACE_MSG_MAX)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->val = val;
> > + __assign_str(dev, info->dev ? netdev_name(info->dev) : "(null)");
> > + __entry->info = info;
> > + __entry->err = err;
> > + switchdev_notifier_str(val, info, __entry->msg, SWITCHDEV_TRACE_MSG_MAX);
>
> Is it possible to just store the information in the trace event and then
> call the above function in the read stage?
I agree with Steven: it looks like that with the above code the
tracepoint itself will become measurably costily in terms of CPU
cycles: we want to avoid that.
Perhaps using different tracepoints with different notifier_block type
would help? so that each trace point could just copy a few specific
fields.
Cheers,
Paolo
Powered by blists - more mailing lists