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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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