* [PATCH] net: phy: add tracepoints
@ 2018-08-16 9:34 Tobias Waldekranz
2018-08-16 9:48 ` Tobias Waldekranz
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Waldekranz @ 2018-08-16 9:34 UTC (permalink / raw)
To: tobias
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Steven Rostedt,
Ingo Molnar, open list, open list:ETHERNET PHY LIBRARY
Two tracepoints for now:
* `phy_interrupt` Pretty self-explanatory.
* `phy_state_change` Whenever the PHY's state machine is run, trace
the old and the new state.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
drivers/net/phy/phy.c | 4 +++
include/trace/events/phy.h | 68 ++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+)
create mode 100644 include/trace/events/phy.h
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 9aabfa1a455a..8d22926f3962 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -38,6 +38,9 @@
#include <asm/irq.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/phy.h>
+
#define PHY_STATE_STR(_state) \
case PHY_##_state: \
return __stringify(_state); \
@@ -1039,6 +1042,7 @@ void phy_state_machine(struct work_struct *work)
if (err < 0)
phy_error(phydev);
+ trace_phy_state_change(phydev, old_state);
if (old_state != phydev->state)
phydev_dbg(phydev, "PHY state change %s -> %s\n",
phy_state_to_str(old_state),
diff --git a/include/trace/events/phy.h b/include/trace/events/phy.h
new file mode 100644
index 000000000000..7ba6c0dda47e
--- /dev/null
+++ b/include/trace/events/phy.h
@@ -0,0 +1,68 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM phy
+
+#if !defined(_TRACE_PHY_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PHY_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(phy_interrupt,
+ TP_PROTO(int irq, struct phy_device *phydev),
+ TP_ARGS(irq, phydev),
+ TP_STRUCT__entry(
+ __field(int, irq)
+ __field(int, addr)
+ __field(int, state)
+ __array(char, ifname, IFNAMSIZ)
+ ),
+ TP_fast_assign(
+ __entry->irq = irq;
+ __entry->addr = phydev->mdio.addr;
+ __entry->state = phydev->state;
+ if (phydev->attached_dev)
+ memcpy(__entry->ifname,
+ netdev_name(phydev->attached_dev),
+ IFNAMSIZ);
+ else
+ memset(__entry->ifname, 0, IFNAMSIZ);
+ ),
+ TP_printk("phy-%d-irq irq=%d ifname=%16s state=%d",
+ __entry->addr,
+ __entry->irq,
+ __entry->ifname,
+ __entry->state
+ )
+ );
+
+TRACE_EVENT(phy_state_change,
+ TP_PROTO(struct phy_device *phydev, enum phy_state old_state),
+ TP_ARGS(phydev, old_state),
+ TP_STRUCT__entry(
+ __field(int, addr)
+ __field(int, state)
+ __field(int, old_state)
+ __array(char, ifname, IFNAMSIZ)
+ ),
+ TP_fast_assign(
+ __entry->addr = phydev->mdio.addr;
+ __entry->state = phydev->state;
+ __entry->old_state = old_state;
+ if (phydev->attached_dev)
+ memcpy(__entry->ifname,
+ netdev_name(phydev->attached_dev),
+ IFNAMSIZ);
+ else
+ memset(__entry->ifname, 0, IFNAMSIZ);
+ ),
+ TP_printk("phy-%d-change ifname=%16s old_state=%d state=%d",
+ __entry->addr,
+ __entry->ifname,
+ __entry->old_state,
+ __entry->state
+ )
+ );
+
+#endif /* _TRACE_PHY_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] net: phy: add tracepoints
2018-08-16 9:34 [PATCH] net: phy: add tracepoints Tobias Waldekranz
@ 2018-08-16 9:48 ` Tobias Waldekranz
0 siblings, 0 replies; 4+ messages in thread
From: Tobias Waldekranz @ 2018-08-16 9:48 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David S. Miller, Steven Rostedt,
Ingo Molnar, open list, open list:ETHERNET PHY LIBRARY
On Thu, Aug 16, 2018 at 11:34:15AM +0200, Tobias Waldekranz wrote:
> Two tracepoints for now:
>
> * `phy_interrupt` Pretty self-explanatory.
>
> * `phy_state_change` Whenever the PHY's state machine is run, trace
> the old and the new state.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> drivers/net/phy/phy.c | 4 +++
> include/trace/events/phy.h | 68 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 72 insertions(+)
> create mode 100644 include/trace/events/phy.h
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 9aabfa1a455a..8d22926f3962 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -38,6 +38,9 @@
>
> #include <asm/irq.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/phy.h>
> +
> #define PHY_STATE_STR(_state) \
> case PHY_##_state: \
> return __stringify(_state); \
> @@ -1039,6 +1042,7 @@ void phy_state_machine(struct work_struct *work)
> if (err < 0)
> phy_error(phydev);
>
> + trace_phy_state_change(phydev, old_state);
> if (old_state != phydev->state)
> phydev_dbg(phydev, "PHY state change %s -> %s\n",
> phy_state_to_str(old_state),
> diff --git a/include/trace/events/phy.h b/include/trace/events/phy.h
> new file mode 100644
> index 000000000000..7ba6c0dda47e
> --- /dev/null
> +++ b/include/trace/events/phy.h
> @@ -0,0 +1,68 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM phy
> +
> +#if !defined(_TRACE_PHY_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_PHY_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(phy_interrupt,
> + TP_PROTO(int irq, struct phy_device *phydev),
> + TP_ARGS(irq, phydev),
> + TP_STRUCT__entry(
> + __field(int, irq)
> + __field(int, addr)
> + __field(int, state)
> + __array(char, ifname, IFNAMSIZ)
> + ),
> + TP_fast_assign(
> + __entry->irq = irq;
> + __entry->addr = phydev->mdio.addr;
> + __entry->state = phydev->state;
> + if (phydev->attached_dev)
> + memcpy(__entry->ifname,
> + netdev_name(phydev->attached_dev),
> + IFNAMSIZ);
> + else
> + memset(__entry->ifname, 0, IFNAMSIZ);
> + ),
> + TP_printk("phy-%d-irq irq=%d ifname=%16s state=%d",
> + __entry->addr,
> + __entry->irq,
> + __entry->ifname,
> + __entry->state
> + )
> + );
> +
> +TRACE_EVENT(phy_state_change,
> + TP_PROTO(struct phy_device *phydev, enum phy_state old_state),
> + TP_ARGS(phydev, old_state),
> + TP_STRUCT__entry(
> + __field(int, addr)
> + __field(int, state)
> + __field(int, old_state)
> + __array(char, ifname, IFNAMSIZ)
> + ),
> + TP_fast_assign(
> + __entry->addr = phydev->mdio.addr;
> + __entry->state = phydev->state;
> + __entry->old_state = old_state;
> + if (phydev->attached_dev)
> + memcpy(__entry->ifname,
> + netdev_name(phydev->attached_dev),
> + IFNAMSIZ);
> + else
> + memset(__entry->ifname, 0, IFNAMSIZ);
> + ),
> + TP_printk("phy-%d-change ifname=%16s old_state=%d state=%d",
> + __entry->addr,
> + __entry->ifname,
> + __entry->old_state,
> + __entry->state
> + )
> + );
> +
> +#endif /* _TRACE_PHY_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> --
> 2.17.1
>
This was sent in error. A v2 will follow. I had not used
get_maintainers.pl before, sorry about the noise!
--
Thanks
- wkz
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] net: phy: add tracepoints
@ 2018-08-16 9:30 Tobias Waldekranz
2018-08-16 12:46 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Waldekranz @ 2018-08-16 9:30 UTC (permalink / raw)
To: Tobias Waldekranz
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Steven Rostedt,
Ingo Molnar, open list, open list:ETHERNET PHY LIBRARY
Two tracepoints for now:
* `phy_interrupt` Pretty self-explanatory.
* `phy_state_change` Whenever the PHY's state machine is run, trace
the old and the new state.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
drivers/net/phy/phy.c | 4 +++
include/trace/events/phy.h | 68 ++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+)
create mode 100644 include/trace/events/phy.h
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 9aabfa1a455a..8d22926f3962 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -38,6 +38,9 @@
#include <asm/irq.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/phy.h>
+
#define PHY_STATE_STR(_state) \
case PHY_##_state: \
return __stringify(_state); \
@@ -1039,6 +1042,7 @@ void phy_state_machine(struct work_struct *work)
if (err < 0)
phy_error(phydev);
+ trace_phy_state_change(phydev, old_state);
if (old_state != phydev->state)
phydev_dbg(phydev, "PHY state change %s -> %s\n",
phy_state_to_str(old_state),
diff --git a/include/trace/events/phy.h b/include/trace/events/phy.h
new file mode 100644
index 000000000000..7ba6c0dda47e
--- /dev/null
+++ b/include/trace/events/phy.h
@@ -0,0 +1,68 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM phy
+
+#if !defined(_TRACE_PHY_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PHY_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(phy_interrupt,
+ TP_PROTO(int irq, struct phy_device *phydev),
+ TP_ARGS(irq, phydev),
+ TP_STRUCT__entry(
+ __field(int, irq)
+ __field(int, addr)
+ __field(int, state)
+ __array(char, ifname, IFNAMSIZ)
+ ),
+ TP_fast_assign(
+ __entry->irq = irq;
+ __entry->addr = phydev->mdio.addr;
+ __entry->state = phydev->state;
+ if (phydev->attached_dev)
+ memcpy(__entry->ifname,
+ netdev_name(phydev->attached_dev),
+ IFNAMSIZ);
+ else
+ memset(__entry->ifname, 0, IFNAMSIZ);
+ ),
+ TP_printk("phy-%d-irq irq=%d ifname=%16s state=%d",
+ __entry->addr,
+ __entry->irq,
+ __entry->ifname,
+ __entry->state
+ )
+ );
+
+TRACE_EVENT(phy_state_change,
+ TP_PROTO(struct phy_device *phydev, enum phy_state old_state),
+ TP_ARGS(phydev, old_state),
+ TP_STRUCT__entry(
+ __field(int, addr)
+ __field(int, state)
+ __field(int, old_state)
+ __array(char, ifname, IFNAMSIZ)
+ ),
+ TP_fast_assign(
+ __entry->addr = phydev->mdio.addr;
+ __entry->state = phydev->state;
+ __entry->old_state = old_state;
+ if (phydev->attached_dev)
+ memcpy(__entry->ifname,
+ netdev_name(phydev->attached_dev),
+ IFNAMSIZ);
+ else
+ memset(__entry->ifname, 0, IFNAMSIZ);
+ ),
+ TP_printk("phy-%d-change ifname=%16s old_state=%d state=%d",
+ __entry->addr,
+ __entry->ifname,
+ __entry->old_state,
+ __entry->state
+ )
+ );
+
+#endif /* _TRACE_PHY_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] net: phy: add tracepoints
2018-08-16 9:30 Tobias Waldekranz
@ 2018-08-16 12:46 ` Steven Rostedt
0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2018-08-16 12:46 UTC (permalink / raw)
To: Tobias Waldekranz
Cc: Tobias Waldekranz, Andrew Lunn, Florian Fainelli, David S. Miller,
Ingo Molnar, open list, open list:ETHERNET PHY LIBRARY
On Thu, 16 Aug 2018 11:30:53 +0200
Tobias Waldekranz <tobias@waldekranz.com> wrote:
> Two tracepoints for now:
>
> * `phy_interrupt` Pretty self-explanatory.
>
> * `phy_state_change` Whenever the PHY's state machine is run, trace
> the old and the new state.
From a tracing perspective, this patch looks fine,
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
But below I have a possible improvement.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> drivers/net/phy/phy.c | 4 +++
> include/trace/events/phy.h | 68 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 72 insertions(+)
> create mode 100644 include/trace/events/phy.h
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 9aabfa1a455a..8d22926f3962 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -38,6 +38,9 @@
>
> #include <asm/irq.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/phy.h>
> +
> #define PHY_STATE_STR(_state) \
> case PHY_##_state: \
> return __stringify(_state); \
> @@ -1039,6 +1042,7 @@ void phy_state_machine(struct work_struct *work)
> if (err < 0)
> phy_error(phydev);
>
> + trace_phy_state_change(phydev, old_state);
I see that you are passing in a enum phy_state.
> if (old_state != phydev->state)
> phydev_dbg(phydev, "PHY state change %s -> %s\n",
> phy_state_to_str(old_state),
> diff --git a/include/trace/events/phy.h b/include/trace/events/phy.h
> new file mode 100644
> index 000000000000..7ba6c0dda47e
> --- /dev/null
> +++ b/include/trace/events/phy.h
> @@ -0,0 +1,68 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM phy
You can add this:
#define PHY_STATE_ENUMS \
EM( DOWN ) \
EM( STARTING ) \
EM( READY ) \
EM( PENDING ) \
EM( UP ) \
EM( AN ) \
EM( RUNNING ) \
EM( NOLINK ) \
EM( FORCING ) \
EM( CHANGELINK )\
EM( HALTED ) \
EMe(RESUMING)
#undef EM
#undef EMe
#define EM(a) TRACE_DEFINE_ENUM( PHY_##a );
#define EMe(a) TRACE_DEFINE_ENUM( PHY_##a );
PHY_STATE_ENUMS
#undef EM
#undef EMe
#define EM(a) { PHY_##a, #a },
#define EMe(a) { PHY_##a, #a }
> +
> +#if !defined(_TRACE_PHY_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_PHY_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(phy_interrupt,
> + TP_PROTO(int irq, struct phy_device *phydev),
> + TP_ARGS(irq, phydev),
> + TP_STRUCT__entry(
> + __field(int, irq)
> + __field(int, addr)
> + __field(int, state)
> + __array(char, ifname, IFNAMSIZ)
> + ),
> + TP_fast_assign(
> + __entry->irq = irq;
> + __entry->addr = phydev->mdio.addr;
> + __entry->state = phydev->state;
> + if (phydev->attached_dev)
> + memcpy(__entry->ifname,
> + netdev_name(phydev->attached_dev),
> + IFNAMSIZ);
> + else
> + memset(__entry->ifname, 0, IFNAMSIZ);
> + ),
> + TP_printk("phy-%d-irq irq=%d ifname=%16s state=%d",
Here you can have "state=%s"
> + __entry->addr,
> + __entry->irq,
> + __entry->ifname,
> + __entry->state
And here you can have:
__print_symbolic(__entry->state,
PHY_STATE_ENUMS )
> + )
> + );
> +
> +TRACE_EVENT(phy_state_change,
> + TP_PROTO(struct phy_device *phydev, enum phy_state old_state),
> + TP_ARGS(phydev, old_state),
> + TP_STRUCT__entry(
> + __field(int, addr)
> + __field(int, state)
> + __field(int, old_state)
> + __array(char, ifname, IFNAMSIZ)
> + ),
> + TP_fast_assign(
> + __entry->addr = phydev->mdio.addr;
> + __entry->state = phydev->state;
> + __entry->old_state = old_state;
> + if (phydev->attached_dev)
> + memcpy(__entry->ifname,
> + netdev_name(phydev->attached_dev),
> + IFNAMSIZ);
> + else
> + memset(__entry->ifname, 0, IFNAMSIZ);
> + ),
> + TP_printk("phy-%d-change ifname=%16s old_state=%d state=%d",
> + __entry->addr,
> + __entry->ifname,
> + __entry->old_state,
> + __entry->state
And do the same above for both old_state and state.
Then instead of just printing numbers and having to remember what state
goes with what number (and god forbid those enums were to change), you
would get actual names of the states, and be easier to read the trace
output.
Note, I didn't test what I wrote, there could be a bug, but it
shouldn't be too far off from what I'm trying to get at.
-- Steve
> + )
> + );
> +
> +#endif /* _TRACE_PHY_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-16 12:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-16 9:34 [PATCH] net: phy: add tracepoints Tobias Waldekranz
2018-08-16 9:48 ` Tobias Waldekranz
-- strict thread matches above, loose matches on Subject: below --
2018-08-16 9:30 Tobias Waldekranz
2018-08-16 12:46 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox