[PATCH 1/2] mtd: Make sure MTD objects always have a valid debugfs dir

Boris Brezillon
2017-10-04T13:53:18Z

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")
Signed-off-by: Boris Brezillon <boris.brezillon@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;

--
2.11.0