diff mbox series

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

Message ID 20230731-cxl-fix-clear-event-debug-print-v2-2-9bdd08d386e9@intel.com
State New, archived
Headers show
Series CXL/events: Fix and improve event dev_dbg() messages | expand

Commit Message

Ira Weiny Aug. 1, 2023, 8:13 p.m. UTC
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 <ira.weiny@intel.com>
---
 drivers/cxl/core/mbox.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Dave Jiang Aug. 1, 2023, 8:41 p.m. UTC | #1
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 <ira.weiny@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   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);
>
Alison Schofield Aug. 2, 2023, 4:29 p.m. UTC | #2
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 <ira.weiny@intel.com>
> ---
>  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
>
Ira Weiny Aug. 3, 2023, 5:05 a.m. UTC | #3
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 <ira.weiny@intel.com>
> > ---
> >  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
> >
Alison Schofield Aug. 3, 2023, 5:26 p.m. UTC | #4
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 <ira.weiny@intel.com>
> > > ---
> > >  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
> > > 
> 
>
Alison Schofield Aug. 3, 2023, 5:26 p.m. UTC | #5
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 <ira.weiny@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>


> ---
>  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
>
Jonathan Cameron Aug. 4, 2023, 12:17 p.m. UTC | #6
On Thu, 3 Aug 2023 10:26:00 -0700
Alison Schofield <alison.schofield@intel.com> 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 <ira.weiny@intel.com>
> > > > ---
> > > >  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 <Jonathan.Cameron@huawei.com>

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

Patch

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);