[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <64f8ebf0d849a_1e8e782946d@iweiny-mobl.notmuch>
Date: Wed, 6 Sep 2023 14:15:28 -0700
From: Ira Weiny <ira.weiny@...el.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Ira Weiny <ira.weiny@...el.com>
CC: Dan Williams <dan.j.williams@...el.com>,
Navneet Singh <navneet.singh@...el.com>,
Fan Ni <fan.ni@...sung.com>,
Davidlohr Bueso <dave@...olabs.net>,
Dave Jiang <dave.jiang@...el.com>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
<linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC v2 16/18] tools/testing/cxl: Make event logs dynamic
Jonathan Cameron wrote:
> On Mon, 28 Aug 2023 22:21:07 -0700
> Ira Weiny <ira.weiny@...el.com> 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@...el.com>
> Diff did a horrible job on readability of this patch.
Yea apologies. I'm not sure if b4 can use --patience or not but I tried
patience by hand and it did not do any better.
>
> 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 +
Done.
>
> > };
> >
>
> ...
>
>
> >
> > -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()?
Yea! Thanks!
Ira
Powered by blists - more mailing lists