Messages in this thread | | | Subject | Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info | From | Yufen Yu <> | Date | Thu, 16 Apr 2020 16:34:13 +0800 |
| |
Hi,
On 2020/4/16 15:15, Christoph Hellwig wrote: > Cache a copy of the name for the life time of the backing_dev_info > structure so that we can reference it even after unregistering. > > Fixes: 68f23b89067f ("memcg: fix a crash in wb_workfn when a device disappears") > Reported-by: Yufen Yu <yuyufen@huawei.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/backing-dev-defs.h | 1 + > mm/backing-dev.c | 13 ++++++++++--- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h > index 4fc87dee005a..249590bcccf7 100644 > --- a/include/linux/backing-dev-defs.h > +++ b/include/linux/backing-dev-defs.h > @@ -220,6 +220,7 @@ struct backing_dev_info { > wait_queue_head_t wb_waitq; > > struct device *dev; > + const char *dev_name; > struct device *owner; > > struct timer_list laptop_mode_wb_timer; > diff --git a/mm/backing-dev.c b/mm/backing-dev.c > index c2c44c89ee5d..4f6c05df72f9 100644 > --- a/mm/backing-dev.c > +++ b/mm/backing-dev.c > @@ -938,9 +938,15 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args) > if (bdi->dev) /* The driver needs to use separate queues per device */ > return 0; > > - dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args); > - if (IS_ERR(dev)) > + bdi->dev_name = kvasprintf(GFP_KERNEL, fmt, args); > + if (!bdi->dev_name) > + return -ENOMEM; > + > + dev = device_create(bdi_class, NULL, MKDEV(0, 0), bdi, bdi->dev_name); > + if (IS_ERR(dev)) { > + kfree(bdi->dev_name); > return PTR_ERR(dev); > + } > > cgwb_bdi_register(bdi); > bdi->dev = dev; > @@ -1034,6 +1040,7 @@ static void release_bdi(struct kref *ref) > WARN_ON_ONCE(bdi->dev); > wb_exit(&bdi->wb); > cgwb_bdi_exit(bdi); > + kfree(bdi->dev_name); > kfree(bdi); > }
When driver try to to re-register bdi but without release_bdi(), the old dev_name will be cover directly by the newer in bdi_register_va(). So, I am not sure whether it can cause memory leak for bdi->dev_name.
Thanks, Yufen
> > @@ -1047,7 +1054,7 @@ const char *bdi_dev_name(struct backing_dev_info *bdi) > { > if (!bdi || !bdi->dev) > return bdi_unknown_name; > - return dev_name(bdi->dev); > + return bdi->dev_name; > } > EXPORT_SYMBOL_GPL(bdi_dev_name);
| |