[PATCH 03/58] mtd: add opt-in lifetime ownership for backing objects

[James Hilliard <james.hilliard1@gmail.com>]

MTD references retain registered nodes, but do not retain the backing
object allocated by a controller driver. Keeping a partition open across
driver unbind can therefore leave its master and driver metadata freed.
Device references held outside get_mtd_device(), such as NVMEM
consumers, also need the backing object and the partition ancestry to
remain valid.

Add an optional master lifetime initialized by mtd_set_release(). The
driver owns the initial reference. Each registered node and acquired MTD
handle holds another reference. The node reference lasts through final
device release, and nested partitions retain their parent device until
then. mtd_put_master() drops the driver's reference after hardware has
been disconnected and the MTD devices unregistered.

Give OTP NVMEM providers their own backing-object references through the
NVMEM context callbacks. They are parented to the physical device rather
than the MTD node, so the node reference does not cover them. Keep these
references through final NVMEM device release, including after failed MTD
registration, and use the backing driver's module as the consumer owner.

Pin the release callback's module when the driver drops its reference.
If the module is initializing or unloading, wait for the remaining
references instead: failed module initialization can discard module text
despite reference pins, and unload cannot acquire a new pin. Keep the
MTD core module pinned through its device and OTP release callbacks too.

Distinguish registration from retained references so an unregistered
node cannot be opened again. Reference a registered master even when
MTD_PARTITIONED_MASTER is disabled: an unpartitioned device is still
registered in that configuration. Remove the lookup entry under
mtd_table_mutex at the last MTD put, but reserve its number until final
device release. A retained NVMEM provider still uses that number in its
device name, so reusing it earlier would make rebind fail with -EEXIST.

On registration failure, wait for the overridden device release callback
before returning ownership to the caller. This also covers delayed
kobject release, when an immediate caller free would be premature.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/mtd/mtdcore.c   | 195 +++++++++++++++++++++++++++++++++++++++++-------
 include/linux/mtd/mtd.h |  16 ++++
 2 files changed, 184 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 558a605ba749..42c06a02cd99 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -31,6 +31,7 @@
 #include <linux/nvmem-provider.h>
 #include <linux/root_dev.h>
 #include <linux/error-injection.h>
+#include <linux/completion.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -70,6 +71,7 @@ static struct class mtd_class = {
 };
 
 static DEFINE_IDR(mtd_idr);
+static DEFINE_IDA(mtd_ids);
 
 /* These are exported solely for the purpose of mtd_blkdevs.c. You
    should not use them for _anything_ else */
@@ -78,7 +80,15 @@ 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;
+
+	while ((mtd = idr_get_next(&mtd_idr, &i))) {
+		if (mtd->registered)
+			return mtd;
+		i++;
+	}
+
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(__mtd_next_device);
 
@@ -87,37 +97,118 @@ static LIST_HEAD(mtd_notifiers);
 
 #define MTD_DEVT(index) MKDEV(MTD_CHAR_MAJOR, (index)*2)
 
-/* REVISIT once MTD uses the driver model better, whoever allocates
- * the mtd_info will probably want to use the release() hook...
+static void mtd_master_release(struct kref *ref)
+{
+	struct mtd_info *master = container_of(ref, struct mtd_info,
+					     master.life_refcnt);
+	struct completion *done = master->master.release_done;
+	struct module *owner = master->master.release_owner;
+
+	master->master.release(master);
+	if (done)
+		complete(done);
+	else
+		module_put(owner);
+}
+
+static void mtd_get_lifetime(struct mtd_info *master)
+{
+	if (master->master.release)
+		kref_get(&master->master.life_refcnt);
+}
+
+static void mtd_put_lifetime(struct mtd_info *master)
+{
+	if (master->master.release)
+		kref_put(&master->master.life_refcnt, mtd_master_release);
+}
+
+/**
+ * mtd_set_release - initialize the lifetime of a dynamically allocated master
+ * @master: master MTD device
+ * @release: free the backing object and its retained metadata
+ *
+ * Call once before registration. The driver owns the initial reference and
+ * drops it with mtd_put_master() after unregistering and disconnecting hardware.
+ * Registered MTD devices and acquired handles retain the backing object too.
+ * The release callback must not access resources released by driver unbind.
  */
+void mtd_set_release(struct mtd_info *master,
+		     void (*release)(struct mtd_info *mtd))
+{
+	kref_init(&master->master.life_refcnt);
+	master->master.release = release;
+}
+EXPORT_SYMBOL_GPL(mtd_set_release);
+
+/**
+ * mtd_put_master - drop the driver's initial lifetime reference
+ * @master: master initialized by mtd_set_release()
+ *
+ * Hardware access must already be disconnected and drained. Unbind may return
+ * while handles retain metadata. Keep the release callback's module loaded
+ * until that metadata is freed. During module unload there can be no acquired
+ * handles, since they pin the module; wait for remaining device references
+ * instead of scheduling a callback into the departing module. Also wait during
+ * module initialization, which may fail and discard the module despite pins.
+ */
+void mtd_put_master(struct mtd_info *master)
+{
+	DECLARE_COMPLETION_ONSTACK(done);
+
+	if ((!master->owner || READ_ONCE(master->owner->state) == MODULE_STATE_LIVE) &&
+	    try_module_get(master->owner)) {
+		master->master.release_owner = master->owner;
+		mtd_put_lifetime(master);
+	} else {
+		master->master.release_done = &done;
+		mtd_put_lifetime(master);
+		wait_for_completion(&done);
+	}
+}
+EXPORT_SYMBOL_GPL(mtd_put_master);
+
 static void mtd_release(struct device *dev)
 {
 	struct mtd_info *mtd = dev_get_drvdata(dev);
-	dev_t index = MTD_DEVT(mtd->index);
+	struct mtd_info *master = mtd->ref_master;
+	struct mtd_info *parent = mtd->parent;
 
-	idr_remove(&mtd_idr, mtd->index);
+	/* Retained NVMEM providers use the MTD number in their device name. */
+	ida_free(&mtd_ids, mtd->index);
 	of_node_put(mtd_get_of_node(mtd));
 
 	if (mtd_is_partition(mtd))
 		release_mtd_partition(mtd);
 
-	/* remove /dev/mtdXro node */
-	device_destroy(&mtd_class, index + 1);
+	if (master) {
+		/* Nested partitions can outlive device_del() of their parents. */
+		if (parent && parent != master)
+			put_device(&parent->dev);
+		mtd_put_lifetime(master);
+	}
+	module_put(THIS_MODULE);
 }
 
 /*
- * No-op device release used in add_mtd_device() error paths.
+ * Hand failed registrations back to their caller only after device release.
  * Prevents mtd_release() from being called via device_release(),
  * which would free the mtd_info that the caller still manages.
  */
-static void mtd_dev_release_nop(struct device *dev)
+static void mtd_dev_release_done(struct device *dev)
 {
+	complete(dev_get_drvdata(dev));
 }
 
 static void mtd_device_release(struct kref *kref)
 {
 	struct mtd_info *mtd = container_of(kref, struct mtd_info, refcnt);
 	bool is_partition = mtd_is_partition(mtd);
+	bool managed = mtd->ref_master;
+
+	/* The table mutex is held by the registration or handle put path. */
+	idr_remove(&mtd_idr, mtd->index);
+	device_destroy(&mtd_class, MTD_DEVT(mtd->index) + 1);
 
 	debugfs_remove_recursive(mtd->dbg.dfs_dir);
 
@@ -131,10 +222,8 @@ static void mtd_device_release(struct kref *kref)
 	 *  Should not be done for partition,
 	 *  as it was already destroyed in device_unregister().
 	 */
-	if (!is_partition)
+	if (!is_partition && !managed)
 		memset(&mtd->dev, 0, sizeof(mtd->dev));
-
-	module_put(THIS_MODULE);
 }
 
 #define MTD_DEVICE_ATTR_RO(name) \
@@ -612,7 +701,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 	config.id = NVMEM_DEVID_NONE;
 	config.dev = &mtd->dev;
 	config.name = dev_name(&mtd->dev);
-	config.owner = THIS_MODULE;
+	config.owner = mtd_get_master(mtd)->owner;
 	config.add_legacy_fixed_of_cells = of_device_is_compatible(node, "nvmem-cells");
 	config.reg_read = mtd_nvmem_reg_read;
 	config.size = mtd->size;
@@ -702,10 +791,13 @@ static void mtd_check_of_node(struct mtd_info *mtd)
 
 int add_mtd_device(struct mtd_info *mtd)
 {
+	DECLARE_COMPLETION_ONSTACK(released);
 	struct device_node *np = mtd_get_of_node(mtd);
 	struct mtd_info *master = mtd_get_master(mtd);
 	struct mtd_notifier *not;
 	int i, error, ofidx;
+	u32 id;
+	bool wait_for_release = false;
 
 	/*
 	 * May occur, for instance, on buggy drivers which call
@@ -748,16 +840,28 @@ int add_mtd_device(struct mtd_info *mtd)
 	if (np)
 		ofidx = of_alias_get_id(np, "mtd");
 	if (ofidx >= 0)
-		i = idr_alloc(&mtd_idr, mtd, ofidx, ofidx + 1, GFP_KERNEL);
+		i = ida_alloc_range(&mtd_ids, ofidx, ofidx, GFP_KERNEL);
 	else
-		i = idr_alloc(&mtd_idr, mtd, 0, 0, GFP_KERNEL);
+		i = ida_alloc(&mtd_ids, GFP_KERNEL);
 	if (i < 0) {
 		error = i;
 		goto fail_locked;
 	}
+	id = i;
+	error = idr_alloc_u32(&mtd_idr, mtd, &id, id, GFP_KERNEL);
+	if (error) {
+		ida_free(&mtd_ids, i);
+		goto fail_locked;
+	}
 
 	mtd->index = i;
 	kref_init(&mtd->refcnt);
+	if (master->master.release) {
+		mtd_get_lifetime(master);
+		mtd->ref_master = master;
+		if (mtd->parent && mtd->parent != master)
+			get_device(&mtd->parent->dev);
+	}
 
 	/* default value if not set by driver */
 	if (mtd->bitflip_threshold == 0)
@@ -821,7 +925,10 @@ int add_mtd_device(struct mtd_info *mtd)
 	device_create(&mtd_class, mtd->dev.parent, MTD_DEVT(i) + 1, NULL,
 		      "mtd%dro", i);
 
+	/* Keep the release callback loaded until the final device reference. */
+	__module_get(THIS_MODULE);
 	pr_debug("mtd: Giving out device %d to %s\n", i, mtd->name);
+	mtd->registered = true;
 	/* 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)
@@ -839,29 +946,38 @@ int add_mtd_device(struct mtd_info *mtd)
 		}
 	}
 
-	/* We _know_ we aren't being removed, because
-	   our caller is still holding us here. So none
-	   of this try_ nonsense, and no bitching about it
-	   either. :) */
-	__module_get(THIS_MODULE);
 	return 0;
 
 fail_nvmem_add:
 	device_del(&mtd->dev);
 fail_added:
 	/*
-	 * Clear type and set nop release to prevent mtd_release() ->
+	 * Clear type and override release to prevent mtd_release() ->
 	 * release_mtd_partition() -> free_partition() from freeing mtd.
 	 * The caller handles cleanup on failure.
 	 */
 	mtd->dev.type = NULL;
-	mtd->dev.release = mtd_dev_release_nop;
+	mtd->dev.release = mtd_dev_release_done;
+	dev_set_drvdata(&mtd->dev, &released);
+	wait_for_release = true;
 	put_device(&mtd->dev);
 	of_node_put(mtd_get_of_node(mtd));
 fail_devname:
 	idr_remove(&mtd_idr, i);
+	ida_free(&mtd_ids, i);
+	if (mtd->ref_master) {
+		mtd->ref_master = NULL;
+		if (mtd->parent && mtd->parent != master)
+			put_device(&mtd->parent->dev);
+		mtd_put_lifetime(master);
+	}
 fail_locked:
 	mutex_unlock(&mtd_table_mutex);
+	if (wait_for_release) {
+		wait_for_completion(&released);
+		mtd->dev.release = NULL;
+		dev_set_drvdata(&mtd->dev, mtd);
+	}
 	return error;
 }
 
@@ -882,7 +998,7 @@ int del_mtd_device(struct mtd_info *mtd)
 
 	mutex_lock(&mtd_table_mutex);
 
-	if (idr_find(&mtd_idr, mtd->index) != mtd) {
+	if (idr_find(&mtd_idr, mtd->index) != mtd || !mtd->registered) {
 		ret = -ENODEV;
 		goto out_error;
 	}
@@ -892,6 +1008,7 @@ int del_mtd_device(struct mtd_info *mtd)
 	list_for_each_entry(not, &mtd_notifiers, list)
 		not->remove(mtd);
 
+	mtd->registered = false;
 	kref_put(&mtd->refcnt, mtd_device_release);
 	ret = 0;
 
@@ -952,6 +1069,18 @@ static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
 	return ret == -ENODATA ? 0 : ret;
 }
 
+static void mtd_otp_nvmem_get(void *priv)
+{
+	mtd_get_lifetime(priv);
+	__module_get(THIS_MODULE);
+}
+
+static void mtd_otp_nvmem_put(void *priv)
+{
+	mtd_put_lifetime(priv);
+	module_put(THIS_MODULE);
+}
+
 static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd,
 						   const char *compatible,
 						   int size,
@@ -968,7 +1097,7 @@ static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd,
 	config.dev = mtd->dev.parent;
 	config.name = compatible;
 	config.id = NVMEM_DEVID_AUTO;
-	config.owner = THIS_MODULE;
+	config.owner = mtd->owner;
 	config.add_legacy_fixed_of_cells = !mtd_type_is_nand(mtd);
 	config.type = NVMEM_TYPE_OTP;
 	config.root_only = true;
@@ -977,6 +1106,10 @@ static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd,
 	config.size = size;
 	config.of_node = np;
 	config.priv = mtd;
+	if (mtd->master.release) {
+		config.priv_get = mtd_otp_nvmem_get;
+		config.priv_put = mtd_otp_nvmem_put;
+	}
 
 	nvmem = nvmem_register(&config);
 	/* Just ignore if there is no NVMEM support in the kernel */
@@ -1336,6 +1469,9 @@ int __get_mtd_device(struct mtd_info *mtd)
 	struct mtd_info *master = mtd_get_master(mtd);
 	int err;
 
+	if (!mtd->registered)
+		return -ENODEV;
+
 	if (master->_get_device) {
 		err = master->_get_device(mtd);
 		if (err)
@@ -1348,13 +1484,14 @@ int __get_mtd_device(struct mtd_info *mtd)
 		return -ENODEV;
 	}
 
+	mtd_get_lifetime(master);
 	while (mtd) {
 		if (mtd != master)
 			kref_get(&mtd->refcnt);
 		mtd = mtd->parent;
 	}
 
-	if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
+	if (master->dev.type)
 		kref_get(&master->refcnt);
 
 	return 0;
@@ -1441,6 +1578,7 @@ void __put_mtd_device(struct mtd_info *mtd)
 	struct mtd_info *master = mtd_get_master(mtd);
 	/* master can be freed in _put_device */
 	struct module *owner = master->owner;
+	bool managed = master->master.release;
 
 	while (mtd) {
 		/* kref_put() can relese mtd, so keep a reference mtd->parent */
@@ -1451,12 +1589,14 @@ void __put_mtd_device(struct mtd_info *mtd)
 		mtd = parent;
 	}
 
-	if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
+	if (master->dev.type)
 		kref_put(&master->refcnt, mtd_device_release);
 
-	/* must be the last use of master */
+	/* Legacy _put_device() callbacks can free an unmanaged master. */
 	if (master->_put_device)
 		master->_put_device(master);
+	if (managed)
+		mtd_put_lifetime(master);
 
 	/* _put_device() runs code from @owner, so drop the module last */
 	module_put(owner);
@@ -2673,6 +2813,7 @@ static void __exit cleanup_mtd(void)
 	bdi_unregister(mtd_bdi);
 	bdi_put(mtd_bdi);
 	idr_destroy(&mtd_idr);
+	ida_destroy(&mtd_ids);
 }
 
 module_init(init_mtd);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 8d10d9d2e830..5a1bdae00665 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -187,6 +187,7 @@ struct mtd_pairing_scheme {
 };
 
 struct module;	/* only needed for owner field in mtd_info */
+struct completion;
 
 /**
  * struct mtd_debug_info - debugging information for an MTD device.
@@ -224,6 +225,10 @@ struct mtd_part {
  *		     not only the master partition list, but also all
  *		     sub-partitions.
  * @suspended: set to 1 when the device is suspended, 0 otherwise
+ * @life_refcnt: optional lifetime reference count for the backing object
+ * @release: free the backing object after its last lifetime reference
+ * @release_owner: module reference held during deferred release
+ * @release_done: completion used when the owner module is already unloading
  *
  * This struct is embedded in mtd_info and contains master-specific
  * properties/fields. The master is the root MTD device from the MTD partition
@@ -233,6 +238,10 @@ struct mtd_master {
 	struct mutex partitions_lock;
 	struct mutex chrdev_lock;
 	unsigned int suspended : 1;
+	struct kref life_refcnt;
+	void (*release)(struct mtd_info *mtd);
+	struct module *release_owner;
+	struct completion *release_done;
 };
 
 struct mtd_info {
@@ -380,6 +389,10 @@ struct mtd_info {
 	struct module *owner;
 	struct device dev;
 	struct kref refcnt;
+	/* Protected by mtd_table_mutex; false after del_mtd_device(). */
+	bool registered;
+	/* Lifetime reference held until the device release callback. */
+	struct mtd_info *ref_master;
 	struct mtd_debug_info dbg;
 	struct nvmem_device *nvmem;
 	struct nvmem_device *otp_user_nvmem;
@@ -681,6 +694,9 @@ extern int mtd_device_parse_register(struct mtd_info *mtd,
 #define mtd_device_register(master, parts, nr_parts)	\
 	mtd_device_parse_register(master, NULL, NULL, parts, nr_parts)
 extern int mtd_device_unregister(struct mtd_info *master);
+void mtd_set_release(struct mtd_info *master,
+		     void (*release)(struct mtd_info *mtd));
+void mtd_put_master(struct mtd_info *master);
 extern struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);
 extern int __get_mtd_device(struct mtd_info *mtd);
 extern void __put_mtd_device(struct mtd_info *mtd);

-- 
2.53.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic