Re: [PATCH V2 09/11] cxl/test: Add generic mock events

Jonathan Cameron
2022-12-01T14:37:38Z

On Wed, 30 Nov 2022 16:27:17 -0800
ira.weiny@intel.com wrote:

> From: Ira Weiny <ira.weiny@intel.com>
>
> Facilitate testing basic Get/Clear Event functionality by creating
> multiple logs and generic events with made up UUID's.
>
> Data is completely made up with data patterns which should be easy to
> spot in trace output.
>
> A single sysfs entry resets the event data and triggers collecting the
> events for testing.
>
> Events are returned one at a time which is within the specification even
> though it does not exercise the full capabilities of what a device may
> do.
>
> Test traces are easy to obtain with a small script such as this:
>
> #!/bin/bash -x
>
> devices=`find /sys/devices/platform -name cxl_mem*`
>
> # Turn on tracing
> echo "" > /sys/kernel/tracing/trace
> echo 1 > /sys/kernel/tracing/events/cxl/enable
> echo 1 > /sys/kernel/tracing/tracing_on
>
> # Generate fake interrupt
> for device in $devices; do
> echo 1 > $device/event_trigger
> done
>
> # Turn off tracing and report events
> echo 0 > /sys/kernel/tracing/tracing_on
> cat /sys/kernel/tracing/trace
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>

A minor comment on xarray cleanup inline

Jonathan

> +void cxl_mock_remove_event_logs(struct device *dev)
> +{
> + struct mock_event_store *mes;
> +
> + mes = xa_erase(&mock_dev_event_store, (unsigned long)dev);

As below, I'd move this into a devm_add_action_or_reset() so
that we don't need to deal with doing it manually.

> +}
> +EXPORT_SYMBOL_GPL(cxl_mock_remove_event_logs);

...

> static int cxl_mock_mem_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct cxl_memdev *cxlmd;
> struct cxl_dev_state *cxlds;
> + u32 ev_status;
> void *lsa;
> int rc;
>
> @@ -281,11 +304,13 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
> if (rc)
> return rc;
>
> + ev_status = cxl_mock_add_event_logs(cxlds);

On error later in this function these leak. Just use devm_add_action_or_reset()
inside cxl_mock_add_event_logs() so we don't have to care about that.

> +
> cxlmd = devm_cxl_add_memdev(cxlds);
> if (IS_ERR(cxlmd))
> return PTR_ERR(cxlmd);
>
> - cxl_mem_get_event_records(cxlds);
> + __cxl_mem_get_event_records(cxlds, ev_status);
>
> if (resource_size(&cxlds->pmem_res) && IS_ENABLED(CONFIG_CXL_PMEM))
> rc = devm_cxl_add_nvdimm(dev, cxlmd);
> @@ -293,6 +318,12 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int cxl_mock_mem_remove(struct platform_device *pdev)
> +{
> + cxl_mock_remove_event_logs(&pdev->dev);
Why not use devm_add_action_or_reset()?

> + return 0;
> +}
> +

>