Messages in this thread | | | Date | Mon, 29 Apr 2024 09:14:21 -0700 | From | Ira Weiny <> | Subject | Re: [PATCH 2/2] cxl/region: Fix missing put_device(region_dev) |
| |
Li Zhijian wrote: > > mutex_lock(&cxlrd->range_lock); > > region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa, > > match_region_by_range); > > if (!region_dev) > > cxlr = construct_region(cxlrd, cxled); > > else > > cxlr = to_cxl_region(region_dev); > > mutex_unlock(&cxlrd->range_lock); > > > > rc = PTR_ERR_OR_ZERO(cxlr); > > if (rc) > > goto out; > > > > if (!region_dev) > > region_dev = &cxlr->dev;
This diff hunk in the commit message is very odd. I would drop it. We know this builds on patch 1 where you made the above change.
> > When to_cxl_region(region_dev) fails, put_device(region_dev) should be > called to decrease the reference count added by device_find_child().
I __think__ this is what Dan was pointing out but I'm not sure.
I wanted to point out that to_cxl_region() can't fail because the device_find_child() is checking that the device is a region device.
Dan, is that what you were saying when you mentioned there were more serious issues if to_cxl_region() were to fail?
Ira
> > Simply put_device(region_dev) if region_dev is valid in the error path. > > Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery") > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > drivers/cxl/core/region.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 3c80aa263a65..75390865382f 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -3117,8 +3117,9 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled) > p->res); > } > > - put_device(region_dev); > out: > + if (region_dev) > + put_device(region_dev); > put_device(cxlrd_dev); > return rc; > } > -- > 2.29.2 >
| |