Messages in this thread | | | Date | Mon, 31 Jul 2023 18:24:02 -0700 | From | Alison Schofield <> | Subject | Re: [PATCH] cxl/mbox: Fix debug message print |
| |
On Mon, Jul 31, 2023 at 01:52:27PM -0700, Ira Weiny wrote:
Looks like this is rolling for the distro list, so..
Please be more specific than this: [PATCH] cxl/mbox: Fix debug message print
Something like this is more useful to folks scanning the one-liners: [PATCH] cxl/mbox: Use correct handle in events debug message
> The handle value used to report an event being cleared by dev_dbg() is > incorrect due to a post increment of the payload handle index. > > Delay the increment of the index until after the print. Also add the > debugging for event processing which was useful in finding this bug. >
"Also" always smells like something that should be a separate patch.
I guess you could rewrite the commit message and keep it in one patch: [PATCH] cxl/mbox: Improve event dev_dbg() messages
Alison
> To: Davidlohr Bueso <dave@stgolabs.net> > To: Jonathan Cameron <jonathan.cameron@huawei.com> > To: Dave Jiang <dave.jiang@intel.com> > To: Alison Schofield <alison.schofield@intel.com> > To: Vishal Verma <vishal.l.verma@intel.com> > To: Dan Williams <dan.j.williams@intel.com> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: linux-cxl@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > NOTE: This does fix a bug in the patch referenced below. However, I > don't think that warrants back porting because this is only a debug > print. > > Fixes: 6ebe28f9ec72 ("cxl/mem: Read, trace, and clear events on driver load") > --- > drivers/cxl/core/mbox.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index d6d067fbee97..f052d5f174ee 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -882,9 +882,10 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds, > */ > i = 0; > for (cnt = 0; cnt < total; cnt++) { > - payload->handles[i++] = get_pl->records[cnt].hdr.handle; > + payload->handles[i] = get_pl->records[cnt].hdr.handle; > dev_dbg(mds->cxlds.dev, "Event log '%d': Clearing %u\n", log, > le16_to_cpu(payload->handles[i])); > + i++; > > if (i == max_handles) { > payload->nr_recs = i; > @@ -946,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); > > --- > base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 > change-id: 20230731-cxl-fix-clear-event-debug-print-3b57da0e956c > > Best regards, > -- > Ira Weiny <ira.weiny@intel.com> >
| |