Hi Alexander, alexander.usyskin@xxxxxxxxx wrote on Tue, 25 Jul 2023 12:50:04 +0000: > Hi > > > > Hi Miquel, > > > > > > Hi Alexander, > > > > > > alexander.usyskin@xxxxxxxxx wrote on Mon, 24 Jul 2023 11:43:59 +0000: > > > > > > > > > > > With this patch applied, when I load up the module, I get the same 3 > > > > > > > > devices: > > > > > > > > /dev/mtd0 > > > > > > > > /dev/mtd0ro > > > > > > > > /dev/mtdblock0 > > > > > > > > > > > > > > > > Upon removal, the below 2 devices still hang around: > > > > > > > > /dev/mtd0 > > > > > > > > /dev/mtd0ro > > > > > > > > > > > > > Our use-case do not produce mtdblock, maybe there are some > > > imbalances > > > > > of get/put? > > > > > > I have somewhere version with pr_debug after every kref_get/put. That > > > may > > > > > help to catch where > > > > > > it missed, I hope. > > > > > > > > > > I believe mtdblock is the good citizen here. Just disable > > > > > CONFIG_MTD_BLOCK from your configuration and you will likely observe > > > > > the same issue, just a bit narrowed, perhaps. Indeed, if you manage to > > > > > follow all the get/put calls it can help to find an imbalance. > > > > > > > > > > Thanks, > > > > > Miquèl > > > > > > > > Miquel, do you have CONFIG_MTD_PARTITIONED_MASTER set in your > > > config? > > > > > > Not sure I get your question. You can enable or disable it, it should > > > work in both cases (yet, the handling is of course a bit different as > > > the top level device will be retained/not retained). > > > > > > Thanks, > > > Miquèl > > > > I'm trying to understand why I can't reproduce the problem in my scenario. > > I found an important difference in upstreamed patch and internal version: > > The IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) check around > > kref_get/put does not exists in the internal tree. > > The code before my patch do not have such check, so I tend to assume that > > this check should be removed. > > If you reproduce happens with CONFIG_MTD_PARTITIONED_MASTER > > disabled that may explain problems that you see. > > > > -- > > Thanks, > > Sasha > > > > I've tried to reproduce this with latest Linux 6.5-rc1 and my two patches. > The manual modprobe mtdblock creates mtdblock0 over my partitions too. > I can't reproduce problem neither with MTD_PARTITIONED_MASTER nor without. > > Let's try to debug on your system, can you enable dynamic debug for mtd subsystem, > reproduce and publish dmesg? > > The prints for kref get/put can be added as below: > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > index 2466ea466466..374835831428 100644 > --- a/drivers/mtd/mtdcore.c > +++ b/drivers/mtd/mtdcore.c > @@ -1242,10 +1242,13 @@ int __get_mtd_device(struct mtd_info *mtd) > } > > kref_get(&mtd->refcnt); > + pr_debug("get mtd %s %d\n", mtd->name, kref_read(&mtd->refcnt)); > > while (mtd->parent) { > - if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd->parent != master) > + if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd->parent != master) { > kref_get(&mtd->parent->refcnt); > + pr_debug("get mtd %s %d\n", mtd->parent->name, kref_read(&mtd->parent->refcnt)); > + } > mtd = mtd->parent; > } > > @@ -1335,12 +1338,15 @@ void __put_mtd_device(struct mtd_info *mtd) > while (mtd != master) { > struct mtd_info *parent = mtd->parent; > > + pr_debug("put mtd %s %d\n", mtd->name, kref_read(&mtd->refcnt)); > kref_put(&mtd->refcnt, mtd_device_release); > mtd = parent; > } > > - if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) > + if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) { > + pr_debug("put mtd %s %d\n", master->name, kref_read(&master->refcnt)); > kref_put(&master->refcnt, mtd_device_release); > + } > > module_put(master->owner); > > Could this be helpful? https://lore.kernel.org/all/20230725215539.3135304-1-zhangxiaoxu5@xxxxxxxxxx/ If you successfully test it, please send your Tested-by. Thanks, Miquèl