2023-08-01 21:19:27

by Ira Weiny

Subject: [PATCH v2 0/2] CXL/events: Fix and improve event dev_dbg() messages

Signed-off-by: Ira Weiny <[email protected]>
---
Changes in v2:
- [Dan] split fix from additional debug message
- Link to v1: https://lore.kernel.org/r/20230731-cxl-fix-clear-event-debug-print-v1-1-42c068f500d1@intel.com

---
Ira Weiny (2):
cxl/mbox: Use correct handle in events debug message
cxl/mbox: Add handle to event processing debug

drivers/cxl/core/mbox.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
---
base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4
change-id: 20230731-cxl-fix-clear-event-debug-print-3b57da0e956c

Best regards,
--
Ira Weiny <[email protected]>

2023-08-01 21:54:37

by Ira Weiny

Subject: [PATCH v2 2/2] cxl/mbox: Add handle to event processing debug

Knowing which handle is being processed helped in debugging new event
code. Add a dev_dbg() message with this information.

Signed-off-by: Ira Weiny <[email protected]>
---
drivers/cxl/core/mbox.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index a1c490f66bbf..f052d5f174ee 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -947,9 +947,13 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
if (!nr_rec)
break;

- for (i = 0; i < nr_rec; i++)
+ for (i = 0; i < nr_rec; i++) {
+ dev_dbg(dev, "Event log %d: processing handle %u\n",
+ type,
+ le16_to_cpu(payload->records[i].hdr.handle));
cxl_event_trace_record(cxlmd, type,
&payload->records[i]);
+ }

if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
trace_cxl_overflow(cxlmd, type, payload);

--
2.41.0

2023-08-01 22:10:20

by Dave Jiang

Subject: Re: [PATCH v2 2/2] cxl/mbox: Add handle to event processing debug

On 8/1/23 13:13, Ira Weiny wrote:
> Knowing which handle is being processed helped in debugging new event
> code. Add a dev_dbg() message with this information.
>
> Signed-off-by: Ira Weiny <[email protected]>

Reviewed-by: Dave Jiang <[email protected]>
> ---
> drivers/cxl/core/mbox.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index a1c490f66bbf..f052d5f174ee 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -947,9 +947,13 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> if (!nr_rec)
> break;
>
> - for (i = 0; i < nr_rec; i++)
> + for (i = 0; i < nr_rec; i++) {
> + dev_dbg(dev, "Event log %d: processing handle %u\n",
> + type,
> + le16_to_cpu(payload->records[i].hdr.handle));
> cxl_event_trace_record(cxlmd, type,
> &payload->records[i]);
> + }
>
> if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> trace_cxl_overflow(cxlmd, type, payload);
>

2023-08-02 18:55:12

by Alison Schofield

Subject: Re: [PATCH v2 2/2] cxl/mbox: Add handle to event processing debug

On Tue, Aug 01, 2023 at 01:13:29PM -0700, Ira Weiny wrote:
> Knowing which handle is being processed helped in debugging new event
> code. Add a dev_dbg() message with this information.
>
> Signed-off-by: Ira Weiny <[email protected]>
> ---
> drivers/cxl/core/mbox.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index a1c490f66bbf..f052d5f174ee 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -947,9 +947,13 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> if (!nr_rec)
> break;
>
> - for (i = 0; i < nr_rec; i++)
> + for (i = 0; i < nr_rec; i++) {
> + dev_dbg(dev, "Event log %d: processing handle %u\n",
> + type,
> + le16_to_cpu(payload->records[i].hdr.handle));
> cxl_event_trace_record(cxlmd, type,
> &payload->records[i]);
> + }

Is dev_dbg() overkill when the info is in the trace event?

>
> if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> trace_cxl_overflow(cxlmd, type, payload);
>
> --
> 2.41.0
>

2023-08-03 05:58:16

by Ira Weiny

Subject: Re: [PATCH v2 2/2] cxl/mbox: Add handle to event processing debug

Alison Schofield wrote:
> On Tue, Aug 01, 2023 at 01:13:29PM -0700, Ira Weiny wrote:
> > Knowing which handle is being processed helped in debugging new event
> > code. Add a dev_dbg() message with this information.
> >
> > Signed-off-by: Ira Weiny <[email protected]>
> > ---
> > drivers/cxl/core/mbox.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > index a1c490f66bbf..f052d5f174ee 100644
> > --- a/drivers/cxl/core/mbox.c
> > +++ b/drivers/cxl/core/mbox.c
> > @@ -947,9 +947,13 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> > if (!nr_rec)
> > break;
> >
> > - for (i = 0; i < nr_rec; i++)
> > + for (i = 0; i < nr_rec; i++) {
> > + dev_dbg(dev, "Event log %d: processing handle %u\n",
> > + type,
> > + le16_to_cpu(payload->records[i].hdr.handle));
> > cxl_event_trace_record(cxlmd, type,
> > &payload->records[i]);
> > + }
>
> Is dev_dbg() overkill when the info is in the trace event?

The trace event will not happen if tracing is not turned on.

This was all part of my testing the DCD events which don't have to, and
likely will not, have tracing on. So this helped to track which handles
were being processed from the cxl-test and qemu layers.

Ira

>
> >
> > if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> > trace_cxl_overflow(cxlmd, type, payload);
> >
> > --
> > 2.41.0
> >

2023-08-03 18:42:50

by Alison Schofield

Subject: Re: [PATCH v2 2/2] cxl/mbox: Add handle to event processing debug

On Wed, Aug 02, 2023 at 10:05:56PM -0700, Ira Weiny wrote:
> Alison Schofield wrote:
> > On Tue, Aug 01, 2023 at 01:13:29PM -0700, Ira Weiny wrote:
> > > Knowing which handle is being processed helped in debugging new event
> > > code. Add a dev_dbg() message with this information.
> > >
> > > Signed-off-by: Ira Weiny <[email protected]>
> > > ---
> > > drivers/cxl/core/mbox.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > > index a1c490f66bbf..f052d5f174ee 100644
> > > --- a/drivers/cxl/core/mbox.c
> > > +++ b/drivers/cxl/core/mbox.c
> > > @@ -947,9 +947,13 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> > > if (!nr_rec)
> > > break;
> > >
> > > - for (i = 0; i < nr_rec; i++)
> > > + for (i = 0; i < nr_rec; i++) {
> > > + dev_dbg(dev, "Event log %d: processing handle %u\n",
> > > + type,
> > > + le16_to_cpu(payload->records[i].hdr.handle));
> > > cxl_event_trace_record(cxlmd, type,
> > > &payload->records[i]);
> > > + }
> >
> > Is dev_dbg() overkill when the info is in the trace event?
>
> The trace event will not happen if tracing is not turned on.
>
> This was all part of my testing the DCD events which don't have to, and
> likely will not, have tracing on. So this helped to track which handles
> were being processed from the cxl-test and qemu layers.
>
> Ira

Thanks for explaining!

>
> >
> > >
> > > if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> > > trace_cxl_overflow(cxlmd, type, payload);
> > >
> > > --
> > > 2.41.0
> > >
>
>

2023-08-03 18:43:13

by Alison Schofield

Subject: Re: [PATCH v2 2/2] cxl/mbox: Add handle to event processing debug

On Tue, Aug 01, 2023 at 01:13:29PM -0700, Ira Weiny wrote:
> Knowing which handle is being processed helped in debugging new event
> code. Add a dev_dbg() message with this information.
>
> Signed-off-by: Ira Weiny <[email protected]>

Reviewed-by: Alison Schofield <[email protected]>


> ---
> drivers/cxl/core/mbox.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index a1c490f66bbf..f052d5f174ee 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -947,9 +947,13 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> if (!nr_rec)
> break;
>
> - for (i = 0; i < nr_rec; i++)
> + for (i = 0; i < nr_rec; i++) {
> + dev_dbg(dev, "Event log %d: processing handle %u\n",
> + type,
> + le16_to_cpu(payload->records[i].hdr.handle));
> cxl_event_trace_record(cxlmd, type,
> &payload->records[i]);
> + }
>
> if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> trace_cxl_overflow(cxlmd, type, payload);
>
> --
> 2.41.0
>

2023-08-04 12:53:32

by Jonathan Cameron

Subject: Re: [PATCH v2 2/2] cxl/mbox: Add handle to event processing debug

On Thu, 3 Aug 2023 10:26:00 -0700
Alison Schofield <[email protected]> wrote:

> On Wed, Aug 02, 2023 at 10:05:56PM -0700, Ira Weiny wrote:
> > Alison Schofield wrote:
> > > On Tue, Aug 01, 2023 at 01:13:29PM -0700, Ira Weiny wrote:
> > > > Knowing which handle is being processed helped in debugging new event
> > > > code. Add a dev_dbg() message with this information.
> > > >
> > > > Signed-off-by: Ira Weiny <[email protected]>
> > > > ---
> > > > drivers/cxl/core/mbox.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > > > index a1c490f66bbf..f052d5f174ee 100644
> > > > --- a/drivers/cxl/core/mbox.c
> > > > +++ b/drivers/cxl/core/mbox.c
> > > > @@ -947,9 +947,13 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> > > > if (!nr_rec)
> > > > break;
> > > >
> > > > - for (i = 0; i < nr_rec; i++)
> > > > + for (i = 0; i < nr_rec; i++) {
> > > > + dev_dbg(dev, "Event log %d: processing handle %u\n",
> > > > + type,
> > > > + le16_to_cpu(payload->records[i].hdr.handle));
> > > > cxl_event_trace_record(cxlmd, type,
> > > > &payload->records[i]);
> > > > + }
> > >
> > > Is dev_dbg() overkill when the info is in the trace event?
> >
> > The trace event will not happen if tracing is not turned on.
> >
> > This was all part of my testing the DCD events which don't have to, and
> > likely will not, have tracing on. So this helped to track which handles
> > were being processed from the cxl-test and qemu layers.
> >
> > Ira
>
> Thanks for explaining!

Good to have that detail in the commit message as it's good justification
for the change. Otherwise LGTM.

Reviewed-by: Jonathan Cameron <[email protected]>

>
> >
> > >
> > > >
> > > > if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> > > > trace_cxl_overflow(cxlmd, type, payload);
> > > >
> > > > --
> > > > 2.41.0
> > > >
> >
> >