Re: [PATCH 16/26] cxl/extent: Realize extent devices

Ira Weiny
2024-04-24T19:58:02Z

Dave Jiang wrote:
>
>
> On 3/24/24 4:18 PM, ira.weiny@intel.com wrote:
> > From: Navneet Singh <navneet.singh@intel.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