Re: [PATCH v7 1/8] cxl/mem: Read, trace, and clear events on driver load

[Ira Weiny <ira.weiny@intel.com>]

Huai-Cheng wrote:
> On Wed, Jan 18, 2023 at 1:53 PM Ira Weiny <ira.weiny@intel.com> wrote:

[snip]

> > +static void cxl_mem_get_records_log(struct cxl_dev_state *cxlds,
> > +                                   enum cxl_event_log_type type)
> > +{
> > +       struct cxl_get_event_payload *payload;
> > +       struct cxl_mbox_cmd mbox_cmd;
> > +       u8 log_type = type;
> > +       u16 nr_rec;
> > +
> > +       mutex_lock(&cxlds->event.log_lock);
> > +       payload = cxlds->event.buf;
> > +
> > +       mbox_cmd = (struct cxl_mbox_cmd) {
> > +               .opcode = CXL_MBOX_OP_GET_EVENT_RECORD,
> > +               .payload_in = &log_type,
> > +               .size_in = sizeof(log_type),
> > +               .payload_out = payload,
> > +               .size_out = cxlds->payload_size,
> > +               .min_out = struct_size(payload, records, 0),
> > +       };
> > +
> > +       do {
> > +               int rc, i;
> > +
> > +               rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
> > +               if (rc) {
> > +                       dev_err_ratelimited(cxlds->dev,
> > +                               "Event log '%d': Failed to query event records : %d",
> > +                               type, rc);
> > +                       break;
> > +               }
> > +
> > +               nr_rec = le16_to_cpu(payload->record_count);
> > +               if (!nr_rec)
> > +                       break;
> > +
> > +               for (i = 0; i < nr_rec; i++)
> > +                       trace_cxl_generic_event(cxlds->dev, type,
> > +                                               &payload->records[i]);
> > +
> > +               if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> > +                       trace_cxl_overflow(cxlds->dev, type, payload);
> > +
> > +               rc = cxl_clear_event_record(cxlds, type, payload);
> > +               if (rc) {
> > +                       dev_err_ratelimited(cxlds->dev,
> > +                               "Event log '%d': Failed to clear events : %d",
> > +                               type, rc);
> > +                       break;
> > +               }
> > +       } while (nr_rec);
> Should the (payload->flags & CXL_GET_EVENT_FLAG_MORE_RECORDS) be used
> instead of (nr_rec) in this while condition? According to the spec,
> this bit is used to see if there
> are more event records.

It is not an error to query for events with no events being present.
Because we want to read all the events we keep reading if there 'may' be
more and don't care about the overhead of an extra read if there is not.

Ira