Re: [PATCH RFC v2 16/18] tools/testing/cxl: Make event logs dynamic

From: Jonathan Cameron
Date: Wed Aug 30 2023 - 15:16:40 EST


On Mon, 28 Aug 2023 22:21:07 -0700
Ira Weiny <ira.weiny@xxxxxxxxx> wrote:

> The test event logs were created as static arrays as an easy way to mock
> events. Dynamic Capacity Device (DCD) test support requires events be
> created dynamically when extents are created/destroyed.
>
> Modify the event log storage to be dynamically allocated. Thus they can
> accommodate the dynamic events required by DCD. Reuse the static event
> data to create the dynamic events in the new logs without inventing
> complex event injection through the test sysfs. Simplify the processing
> of the logs by using the event log array index as the handle. Add a
> lock to manage concurrency to come with DCD extent testing.
>
> Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
Diff did a horrible job on readability of this patch.

Ah well. Comments superficial only.

Jonathan

> ---
> tools/testing/cxl/test/mem.c | 276 ++++++++++++++++++++++++++-----------------
> 1 file changed, 170 insertions(+), 106 deletions(-)
>
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index 51be202fabd0..6a036c8d215d 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -118,18 +118,27 @@ static struct {
>
> #define PASS_TRY_LIMIT 3
>
> -#define CXL_TEST_EVENT_CNT_MAX 15
> +#define CXL_TEST_EVENT_CNT_MAX 17
>
> /* Set a number of events to return at a time for simulation. */
> #define CXL_TEST_EVENT_CNT 3
>
> +/*
> + * @next_handle: next handle (index) to be stored to
> + * @cur_handle: current handle (index) to be returned to the user on get_event
> + * @nr_events: total events in this log
> + * @nr_overflow: number of events added past the log size
> + * @lock: protect these state variables
> + * @events: array of pending events to be returned.
> + */
> struct mock_event_log {
> - u16 clear_idx;
> - u16 cur_idx;
> + u16 next_handle;
> + u16 cur_handle;
> u16 nr_events;
> u16 nr_overflow;
> - u16 overflow_reset;
> - struct cxl_event_record_raw *events[CXL_TEST_EVENT_CNT_MAX];
> + rwlock_t lock;
> + /* 1 extra slot to accommodate that handles can't be 0 */
> + struct cxl_event_record_raw *events[CXL_TEST_EVENT_CNT_MAX+1];

Spaces around +

> };
>

...


>
> -static void cxl_mock_add_event_logs(struct mock_event_store *mes)
> +/* Create a dynamically allocated event out of a statically defined event. */
> +static void add_event_from_static(struct mock_event_store *mes,
> + enum cxl_event_log_type log_type,
> + struct cxl_event_record_raw *raw)
> +{
> + struct device *dev = mes->mds->cxlds.dev;
> + struct cxl_event_record_raw *rec;
> +
> + rec = devm_kzalloc(dev, sizeof(*rec), GFP_KERNEL);
> + if (!rec) {
> + dev_err(dev, "Failed to alloc event for log\n");
> + return;
> + }
> +
> + memcpy(rec, raw, sizeof(*rec));

devm_kmemdup()?


> + mes_add_event(mes, log_type, rec);
> +}
> +
> +static void cxl_mock_add_event_logs(struct cxl_mockmem_data *mdata)
> {
> + struct mock_event_store *mes = &mdata->mes;
> + struct device *dev = mes->mds->cxlds.dev;
> +
> put_unaligned_le16(CXL_GMER_VALID_CHANNEL | CXL_GMER_VALID_RANK,
> &gen_media.validity_flags);
>
> @@ -438,43 +475,60 @@ static void cxl_mock_add_event_logs(struct mock_event_store *mes)
> CXL_DER_VALID_BANK | CXL_DER_VALID_COLUMN,
> &dram.validity_flags);
>
> - mes_add_event(mes, CXL_EVENT_TYPE_INFO, &maint_needed);
> - mes_add_event(mes, CXL_EVENT_TYPE_INFO,
> + dev_dbg(dev, "Generating fake event logs %d\n",
> + CXL_EVENT_TYPE_INFO);
> + add_event_from_static(mes, CXL_EVENT_TYPE_INFO, &maint_needed);
> + add_event_from_static(mes, CXL_EVENT_TYPE_INFO,
> (struct cxl_event_record_raw *)&gen_media);
> - mes_add_event(mes, CXL_EVENT_TYPE_INFO,
> + add_event_from_static(mes, CXL_EVENT_TYPE_INFO,
> (struct cxl_event_record_raw *)&mem_module);
> mes->ev_status |= CXLDEV_EVENT_STATUS_INFO;
>
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &maint_needed);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL,
> + dev_dbg(dev, "Generating fake event logs %d\n",
> + CXL_EVENT_TYPE_FAIL);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &maint_needed);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL,
> + (struct cxl_event_record_raw *)&mem_module);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL,
> (struct cxl_event_record_raw *)&dram);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL,
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL,
> (struct cxl_event_record_raw *)&gen_media);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL,
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL,
> (struct cxl_event_record_raw *)&mem_module);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL,
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL,
> (struct cxl_event_record_raw *)&dram);
> /* Overflow this log */
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FAIL, &hardware_replace);
> mes->ev_status |= CXLDEV_EVENT_STATUS_FAIL;
>
> - mes_add_event(mes, CXL_EVENT_TYPE_FATAL, &hardware_replace);
> - mes_add_event(mes, CXL_EVENT_TYPE_FATAL,
> + dev_dbg(dev, "Generating fake event logs %d\n",
> + CXL_EVENT_TYPE_FATAL);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FATAL, &hardware_replace);
> + add_event_from_static(mes, CXL_EVENT_TYPE_FATAL,
> (struct cxl_event_record_raw *)&dram);
> mes->ev_status |= CXLDEV_EVENT_STATUS_FATAL;
> }
>
> +static void cxl_mock_event_trigger(struct device *dev)
> +{
> + struct cxl_mockmem_data *mdata = dev_get_drvdata(dev);
> + struct mock_event_store *mes = &mdata->mes;
> +
> + cxl_mock_add_event_logs(mdata);
> + cxl_mem_get_event_records(mes->mds, mes->ev_status);
> +}