[PATCH 1/2] mtd: Make sure MTD objects always have a valid debugfs dir
Boris Brezillon
boris.brezillon at free-electrons.com
Wed Oct 4 06:56:53 PDT 2017
On Wed, 4 Oct 2017 15:53:42 +0200
Boris Brezillon <boris.brezillon at free-electrons.com> wrote:
> On Wed, 4 Oct 2017 15:52:38 +0200
> Boris Brezillon <boris.brezillon at free-electrons.com> wrote:
>
> > Master MTD devices are not registered to MTD subsystem if they are
> > exposing MTD partitions unless the CONFIG_MTD_PARTITIONED_MASTER option
> > is enabled.
> >
> > This lead to a weird situation where some MTD device drivers are trying
> > to add debugfs files to the master MTD device, but this device has no
> > valid debugfs directory.
> >
> > Rework the core logic to do most of the MTD registration steps
> > (including debugfs dir creation) except the registration to the device
> > model, so that master devices are never exposed to the outside world
> > but are still able to expose debugfs entries.
> >
> > These devices will be exposed as mtd-hiddenX in the mtd debugfs dir.
> > Note that X is unique within the hidden MTD device pool but can collide
> > with ids of exposed MTD devs.
> >
> > This commit fixes a bug introduced by commit e8e3edb95ce6 ("mtd: create
> > per-device and module-scope debugfs entries") which is preventing
> > nandsim from loading when CONFIG_DEBUG_FS is enabled.
> >
> > Fixes: e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs entries")
>
> Oops, I forgot
>
> Reported-by: Richard Weinberger <richard at nod.at>
and
Cc: <stable at vger.kernel.org>
:-/
>
> > Signed-off-by: Boris Brezillon <boris.brezillon at free-electrons.com>
> > ---
> > drivers/mtd/mtdcore.c | 83 ++++++++++++++++++++++++++++++++-----------------
> > include/linux/mtd/mtd.h | 1 +
> > 2 files changed, 56 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> > index e7ea842ba3db..323e621f2eba 100644
> > --- a/drivers/mtd/mtdcore.c
> > +++ b/drivers/mtd/mtdcore.c
> > @@ -79,6 +79,10 @@ static struct class mtd_class = {
> > .pm = MTD_CLS_PM_OPS,
> > };
> >
> > +#define MTD_MAX_IDS 0x10000
> > +#define MTD_ID_START 0x0
> > +#define MTD_HIDDEN_ID_START (MTD_ID_START + MTD_MAX_IDS)
> > +
> > static DEFINE_IDR(mtd_idr);
> >
> > /* These are exported solely for the purpose of mtd_blkdevs.c. You
> > @@ -88,7 +92,13 @@ EXPORT_SYMBOL_GPL(mtd_table_mutex);
> >
> > struct mtd_info *__mtd_next_device(int i)
> > {
> > - return idr_get_next(&mtd_idr, &i);
> > + struct mtd_info *mtd;
> > +
> > + mtd = idr_get_next(&mtd_idr, &i);
> > + if (!mtd || i >= MTD_MAX_IDS)
> > + return NULL;
> > +
> > + return mtd;
> > }
> > EXPORT_SYMBOL_GPL(__mtd_next_device);
> >
> > @@ -505,7 +515,13 @@ int add_mtd_device(struct mtd_info *mtd)
> > BUG_ON(mtd->writesize == 0);
> > mutex_lock(&mtd_table_mutex);
> >
> > - i = idr_alloc(&mtd_idr, mtd, 0, 0, GFP_KERNEL);
> > + if (!mtd->hidden)
> > + i = idr_alloc(&mtd_idr, mtd, MTD_ID_START, MTD_MAX_IDS,
> > + GFP_KERNEL);
> > + else
> > + i = idr_alloc(&mtd_idr, mtd, MTD_HIDDEN_ID_START,
> > + MTD_HIDDEN_ID_START + MTD_MAX_IDS,
> > + GFP_KERNEL);
> > if (i < 0) {
> > error = i;
> > goto fail_locked;
> > @@ -545,15 +561,20 @@ int add_mtd_device(struct mtd_info *mtd)
> > /* Caller should have set dev.parent to match the
> > * physical device, if appropriate.
> > */
> > - mtd->dev.type = &mtd_devtype;
> > - mtd->dev.class = &mtd_class;
> > - mtd->dev.devt = MTD_DEVT(i);
> > - dev_set_name(&mtd->dev, "mtd%d", i);
> > dev_set_drvdata(&mtd->dev, mtd);
> > of_node_get(mtd_get_of_node(mtd));
> > - error = device_register(&mtd->dev);
> > - if (error)
> > - goto fail_added;
> > + if (!mtd->hidden) {
> > + mtd->dev.type = &mtd_devtype;
> > + mtd->dev.class = &mtd_class;
> > + mtd->dev.devt = MTD_DEVT(i);
> > + dev_set_name(&mtd->dev, "mtd%d", i);
> > + error = device_register(&mtd->dev);
> > + if (error)
> > + goto fail_added;
> > + } else {
> > + dev_set_name(&mtd->dev, "mtdmaster%d",
> > + i - MTD_HIDDEN_ID_START);
> > + }
> >
> > if (!IS_ERR_OR_NULL(dfs_dir_mtd)) {
> > mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(&mtd->dev), dfs_dir_mtd);
> > @@ -563,14 +584,16 @@ int add_mtd_device(struct mtd_info *mtd)
> > }
> > }
> >
> > - device_create(&mtd_class, mtd->dev.parent, MTD_DEVT(i) + 1, NULL,
> > - "mtd%dro", i);
> > + if (!mtd->hidden) {
> > + device_create(&mtd_class, mtd->dev.parent, MTD_DEVT(i) + 1,
> > + NULL, "mtd%dro", i);
> >
> > - pr_debug("mtd: Giving out device %d to %s\n", i, mtd->name);
> > - /* No need to get a refcount on the module containing
> > - the notifier, since we hold the mtd_table_mutex */
> > - list_for_each_entry(not, &mtd_notifiers, list)
> > - not->add(mtd);
> > + pr_debug("mtd: Giving out device %d to %s\n", i, mtd->name);
> > + /* No need to get a refcount on the module containing
> > + the notifier, since we hold the mtd_table_mutex */
> > + list_for_each_entry(not, &mtd_notifiers, list)
> > + not->add(mtd);
> > + }
> >
> > mutex_unlock(&mtd_table_mutex);
> > /* We _know_ we aren't being removed, because
> > @@ -612,17 +635,20 @@ int del_mtd_device(struct mtd_info *mtd)
> > goto out_error;
> > }
> >
> > - /* No need to get a refcount on the module containing
> > - the notifier, since we hold the mtd_table_mutex */
> > - list_for_each_entry(not, &mtd_notifiers, list)
> > - not->remove(mtd);
> > + if (!mtd->hidden) {
> > + /* No need to get a refcount on the module containing
> > + the notifier, since we hold the mtd_table_mutex */
> > + list_for_each_entry(not, &mtd_notifiers, list)
> > + not->remove(mtd);
> > + }
> >
> > if (mtd->usecount) {
> > printk(KERN_NOTICE "Removing MTD device #%d (%s) with use count %d\n",
> > mtd->index, mtd->name, mtd->usecount);
> > ret = -EBUSY;
> > } else {
> > - device_unregister(&mtd->dev);
> > + if (!mtd->hidden)
> > + device_unregister(&mtd->dev);
> >
> > idr_remove(&mtd_idr, mtd->index);
> > of_node_put(mtd_get_of_node(mtd));
> > @@ -643,15 +669,16 @@ static int mtd_add_device_partitions(struct mtd_info *mtd,
> > int nbparts = parts->nr_parts;
> > int ret;
> >
> > - if (nbparts == 0 || IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) {
> > - ret = add_mtd_device(mtd);
> > - if (ret)
> > - return ret;
> > - }
> > + if (nbparts && !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
> > + mtd->hidden = true;
> > +
> > + ret = add_mtd_device(mtd);
> > + if (ret)
> > + return ret;
> >
> > if (nbparts > 0) {
> > ret = add_mtd_partitions(mtd, real_parts, nbparts);
> > - if (ret && IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
> > + if (ret)
> > del_mtd_device(mtd);
> > return ret;
> > }
> > @@ -857,7 +884,7 @@ struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num)
> > break;
> > }
> > }
> > - } else if (num >= 0) {
> > + } else if (num >= MTD_ID_START && num < MTD_MAX_IDS) {
> > ret = idr_find(&mtd_idr, num);
> > if (mtd && mtd != ret)
> > ret = NULL;
> > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> > index 6cd0f6b7658b..382c996b51df 100644
> > --- a/include/linux/mtd/mtd.h
> > +++ b/include/linux/mtd/mtd.h
> > @@ -268,6 +268,7 @@ struct mtd_info {
> > unsigned int bitflip_threshold;
> >
> > // Kernel-only stuff starts here.
> > + bool hidden;
> > const char *name;
> > int index;
> >
>
More information about the linux-mtd
mailing list