[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221201143727.0000255c@Huawei.com>
Date: Thu, 1 Dec 2022 14:37:27 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: <ira.weiny@...el.com>
CC: Dan Williams <dan.j.williams@...el.com>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
"Ben Widawsky" <bwidawsk@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>,
Davidlohr Bueso <dave@...olabs.net>,
Dave Jiang <dave.jiang@...el.com>,
<linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>
Subject: Re: [PATCH V2 09/11] cxl/test: Add generic mock events
On Wed, 30 Nov 2022 16:27:17 -0800
ira.weiny@...el.com wrote:
> From: Ira Weiny <ira.weiny@...el.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@...el.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;
> +}
> +
>
Powered by blists - more mailing lists