linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: linux-mtd@lists.infradead.org
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Simon Arlott <simon@fire.lp0.eu>
Subject: [PATCH v3 6/6] mtd: partitions: support a cleanup callback for parsers
Date: Wed, 9 Dec 2015 10:24:03 -0800	[thread overview]
Message-ID: <20151209182403.GW120110@google.com> (raw)
In-Reply-To: <1449271518-118900-7-git-send-email-computersforpeace@gmail.com>

If partition parsers need to clean up their resources, we shouldn't
assume that all memory will fit in a single kmalloc() that the caller
can kfree(). We should allow the parser to provide a proper cleanup
routine.

Note that this means we need to keep a hold on the parser's module for a
bit longer, and release it later with mtd_part_parser_put().

Alongside this, define a default callback that we'll automatically use
if the parser doesn't provide one, so we can still retain the old
behavior.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
v3:
 * drop redundant 'if (parsed.parser)' condition before cleanup

v2:
 * put more common logic in mtd_part_parser_cleanup()
 * provide default cleanup routine (i.e., kfree()) for parsers

 drivers/mtd/mtdcore.c          |  3 +--
 drivers/mtd/mtdcore.h          |  2 ++
 drivers/mtd/mtdpart.c          | 35 +++++++++++++++++++++++++++++++++--
 include/linux/mtd/partitions.h |  1 +
 4 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 20b2b38247b6..89d811e7b04a 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -630,8 +630,7 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 
 out:
 	/* Cleanup any parsed partitions */
-	if (parsed.parser)
-		kfree(parsed.parts);
+	mtd_part_parser_cleanup(&parsed);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(mtd_device_parse_register);
diff --git a/drivers/mtd/mtdcore.h b/drivers/mtd/mtdcore.h
index ce81cc2002f4..55fdb8e1fd2a 100644
--- a/drivers/mtd/mtdcore.h
+++ b/drivers/mtd/mtdcore.h
@@ -17,6 +17,8 @@ int parse_mtd_partitions(struct mtd_info *master, const char * const *types,
 			 struct mtd_partitions *pparts,
 			 struct mtd_part_parser_data *data);
 
+void mtd_part_parser_cleanup(struct mtd_partitions *parts);
+
 int __init init_mtdchar(void);
 void __exit cleanup_mtdchar(void);
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 53517d7653cb..10bf304027dd 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -709,10 +709,23 @@ static inline void mtd_part_parser_put(const struct mtd_part_parser *p)
 	module_put(p->owner);
 }
 
+/*
+ * Many partition parsers just expected the core to kfree() all their data in
+ * one chunk. Do that by default.
+ */
+static void mtd_part_parser_cleanup_default(const struct mtd_partition *pparts,
+					    int nr_parts)
+{
+	kfree(pparts);
+}
+
 int __register_mtd_parser(struct mtd_part_parser *p, struct module *owner)
 {
 	p->owner = owner;
 
+	if (!p->cleanup)
+		p->cleanup = &mtd_part_parser_cleanup_default;
+
 	spin_lock(&part_parser_lock);
 	list_add(&p->list, &part_parsers);
 	spin_unlock(&part_parser_lock);
@@ -756,7 +769,9 @@ static const char * const default_mtd_part_types[] = {
  * This function may return:
  * o a negative error code in case of failure
  * o zero otherwise, and @pparts will describe the partitions, number of
- *   partitions, and the parser which parsed them
+ *   partitions, and the parser which parsed them. Caller must release
+ *   resources with mtd_part_parser_cleanup() when finished with the returned
+ *   data.
  */
 int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 			 struct mtd_partitions *pparts,
@@ -780,7 +795,6 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 		ret = (*parser->parse_fn)(master, &pparts->parts, data);
 		pr_debug("%s: parser %s: %i\n",
 			 master->name, parser->name, ret);
-		mtd_part_parser_put(parser);
 		if (ret > 0) {
 			printk(KERN_NOTICE "%d %s partitions found on MTD device %s\n",
 			       ret, parser->name, master->name);
@@ -788,6 +802,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 			pparts->parser = parser;
 			return 0;
 		}
+		mtd_part_parser_put(parser);
 		/*
 		 * Stash the first error we see; only report it if no parser
 		 * succeeds
@@ -798,6 +813,22 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
 	return err;
 }
 
+void mtd_part_parser_cleanup(struct mtd_partitions *parts)
+{
+	const struct mtd_part_parser *parser;
+
+	if (!parts)
+		return;
+
+	parser = parts->parser;
+	if (parser) {
+		if (parser->cleanup)
+			parser->cleanup(parts->parts, parts->nr_parts);
+
+		mtd_part_parser_put(parser);
+	}
+}
+
 int mtd_is_partition(const struct mtd_info *mtd)
 {
 	struct mtd_part *part;
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index cceaf7bd1537..70736e1e6c8f 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -71,6 +71,7 @@ struct mtd_part_parser {
 	const char *name;
 	int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
 			struct mtd_part_parser_data *);
+	void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
 };
 
 /* Container for passing around a set of parsed partitions */
-- 
2.6.0.rc2.230.g3dd15c0

  parent reply	other threads:[~2015-12-09 18:24 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-04 23:25 [PATCH v2 0/6] mtd: partitions: support cleanup callback for parsers Brian Norris
2015-12-04 23:25 ` [PATCH v2 1/6] mtd: ofpart: assign return argument exactly once Brian Norris
2015-12-04 23:57   ` Boris Brezillon
2015-12-04 23:25 ` [PATCH v2 2/6] mtd: partitions: make parsers return 'const' partition arrays Brian Norris
2015-12-04 23:58   ` Boris Brezillon
2015-12-04 23:25 ` [PATCH v2 3/6] mtd: partitions: rename MTD parser get/put Brian Norris
2015-12-05  0:00   ` Boris Brezillon
2015-12-05  0:02     ` Brian Norris
2015-12-04 23:25 ` [PATCH v2 4/6] mtd: partitions: remove kmemdup() Brian Norris
2015-12-05  0:00   ` Boris Brezillon
2015-12-04 23:25 ` [PATCH v2 5/6] mtd: partitions: pass around 'mtd_partitions' wrapper struct Brian Norris
2015-12-05  0:30   ` Boris Brezillon
2015-12-05  0:41     ` Boris Brezillon
2015-12-05  1:45     ` Brian Norris
2015-12-05  4:18       ` Brian Norris
2015-12-05  8:18         ` Boris Brezillon
2015-12-05  8:27       ` Boris Brezillon
2015-12-04 23:25 ` [PATCH v2 6/6] mtd: partitions: support a cleanup callback for parsers Brian Norris
2015-12-05  0:33   ` Boris Brezillon
2015-12-09 18:24   ` Brian Norris [this message]
2015-12-09 21:46     ` [PATCH v3 " Boris Brezillon
2015-12-09 23:00       ` Brian Norris
2015-12-09 18:25 ` [PATCH v2 0/6] mtd: partitions: support " Brian Norris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151209182403.GW120110@google.com \
    --to=computersforpeace@gmail.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=simon@fire.lp0.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).