Re: [PATCH v4 25/28] cxl/region: Read existing extents on region creation

Ira Weiny
2024-10-24T01:41:17Z

Jonathan Cameron wrote:
> On Mon, 07 Oct 2024 18:16:31 -0500
> ira.weiny@intel.com wrote:
>
> > From: Navneet Singh <navneet.singh@intel.com>
> >

[snip]

> >
> One buglet, and a request for an error message.
> With those.
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks.

>
> > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > index d66beec687a0..6b25d15403a3 100644
> > --- a/drivers/cxl/core/mbox.c
> > +++ b/drivers/cxl/core/mbox.c
> > @@ -1697,6 +1697,111 @@ int cxl_dev_dynamic_capacity_identify(struct cxl_memdev_state *mds)
> > }
> > EXPORT_SYMBOL_NS_GPL(cxl_dev_dynamic_capacity_identify, CXL);
> >
> > +/* Return -EAGAIN if the extent list changes while reading */
> > +static int __cxl_process_extent_list(struct cxl_endpoint_decoder *cxled)
> > +{
> > + u32 current_index, total_read, total_expected, initial_gen_num;
> > + struct cxl_memdev_state *mds = cxled_to_mds(cxled);
> > + struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
> > + struct device *dev = mds->cxlds.dev;
> > + struct cxl_mbox_cmd mbox_cmd;
> > + u32 max_extent_count;
> > + bool first = true;
> > +
> > + struct cxl_mbox_get_extent_out *extents __free(kfree) =
>
> __free(kvfree)

Yep fixed

>
> > + kvmalloc(cxl_mbox->payload_size, GFP_KERNEL);
> > + if (!extents)
> > + return -ENOMEM;
>
> ...
>
>
> > +}
>
> > static void cxlr_dax_unregister(void *_cxlr_dax)
> > {
> > struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> > @@ -3224,6 +3233,9 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> > dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
> > dev_name(dev));
> >
> > + if (cxlr->mode == CXL_REGION_DC)
> > + cxlr_add_existing_extents(cxlr);
>
> Whilst there isn't a whole lot we can do if this fails, I'd like an error
> print to indicate something odd is going on. Probably pass any error
> up to here then print a message before carrying on.

Bubbled up an error and added some dev_err() calls. I don't think they
need to be rate limited since regions are not created very often.

Ira

[snip]