[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66296441d655a_e9f9f29497@iweiny-mobl.notmuch>
Date: Wed, 24 Apr 2024 12:57:53 -0700
From: Ira Weiny <ira.weiny@...el.com>
To: Dave Jiang <dave.jiang@...el.com>, <ira.weiny@...el.com>, Fan Ni
<fan.ni@...sung.com>, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
"Navneet Singh" <navneet.singh@...el.com>
CC: Dan Williams <dan.j.williams@...el.com>, Davidlohr Bueso
<dave@...olabs.net>, Alison Schofield <alison.schofield@...el.com>, "Vishal
Verma" <vishal.l.verma@...el.com>, <linux-btrfs@...r.kernel.org>,
<linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 16/26] cxl/extent: Realize extent devices
Dave Jiang wrote:
>
>
> On 3/24/24 4:18 PM, ira.weiny@...el.com wrote:
> > From: Navneet Singh <navneet.singh@...el.com>
> >
[snip]
> > diff --git a/drivers/cxl/core/extent.c b/drivers/cxl/core/extent.c
> > new file mode 100644
> > index 000000000000..487c220f1c3c
> > --- /dev/null
> > +++ b/drivers/cxl/core/extent.c
> > @@ -0,0 +1,133 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> > +
> > +#include <linux/device.h>
> > +#include <linux/slab.h>
> > +#include <cxl.h>
> > +
> > +static DEFINE_IDA(cxl_extent_ida);
>
> According to Documentation/core-api/idr.rst, IDR interface is deprecated and
> xarray usage is preferred.
IDA != IDR
ida_alloc() provides a unique, unused id for the device. I worked hard to
eliminate all extra references to the extent objects so as to ensure object
lifetimes. So I'm keeping this for now.
> > +
> > +static ssize_t offset_show(struct device *dev, struct device_attribute *attr,
> > + char *buf)
>
> Parameter alignment a bit off here? and some of the other functions as well.
Thanks, fixed.
[snip]
> > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > index 9e33a0976828..6b00e717e42b 100644
> > --- a/drivers/cxl/core/mbox.c
> > +++ b/drivers/cxl/core/mbox.c
> > @@ -1020,6 +1020,32 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds,
> > return rc;
> > }
> >
> > +static int cxl_send_dc_cap_response(struct cxl_memdev_state *mds,
> > + struct range *extent, int opcode)
> > +{
> > + struct cxl_mbox_cmd mbox_cmd;
> > + size_t size;
> > +
> > + struct cxl_mbox_dc_response *dc_res __free(kfree);
> > + size = struct_size(dc_res, extent_list, 1);
> > + dc_res = kzalloc(size, GFP_KERNEL);
> > + if (!dc_res)
> > + return -ENOMEM;
> > +
> > + dc_res->extent_list[0].dpa_start = cpu_to_le64(extent->start);
> > + memset(dc_res->extent_list[0].reserved, 0, 8);
>
> Not needed. kzalloc already zeroed.
Thanks, Fan mentioned it too.
Ira
Powered by blists - more mailing lists