From: Richard Weinberger <richard@nod.at>
To: "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
Brian Norris <computersforpeace@gmail.com>
Subject: Race-free NAND device removal
Date: Sun, 3 Jul 2016 15:38:42 +0200 [thread overview]
Message-ID: <57791562.2020703@nod.at> (raw)
Hi!
While working on nandsim I realized that nand_release() ignores the return
value from mtd_device_unregister().
That means NAND devices cannot removed in a race-free manner.
Consider a NAND driver that registers ->_get_device() and ->_put_device()
callbacks for refcounting. In its removal function it will return -EBUSY
whenever the refcount is > 0.
But when device is claimed while removing it, it can happen that the refcount
increments after the check.
MTD can deal with that and mtd_device_unregister() will return EBUSY.
But nand_release() won't notice and the NAND driver continues with the tear down
process.
Would be a change like the following one acceptable or is a NAND driver
allowed to call mtd_device_unregister() itself?
AFAICT the additional call to mtd_device_unregister() in nand_release() would
be an nop then.
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 0b0dc29..dc76bc6 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4604,16 +4604,19 @@ EXPORT_SYMBOL(nand_scan);
* nand_release - [NAND Interface] Free resources held by the NAND device
* @mtd: MTD device structure
*/
-void nand_release(struct mtd_info *mtd)
+int nand_release(struct mtd_info *mtd)
{
+ int ret;
struct nand_chip *chip = mtd_to_nand(mtd);
+ ret = mtd_device_unregister(mtd);
+ if (ret)
+ return ret;
+
if (chip->ecc.mode == NAND_ECC_SOFT &&
chip->ecc.algo == NAND_ECC_BCH)
nand_bch_free((struct nand_bch_control *)chip->ecc.priv);
- mtd_device_unregister(mtd);
-
/* Free bad block table memory */
kfree(chip->bbt);
if (!(chip->options & NAND_OWN_BUFFERS))
@@ -4623,6 +4626,8 @@ void nand_release(struct mtd_info *mtd)
if (chip->badblock_pattern && chip->badblock_pattern->options
& NAND_BBT_DYNAMICSTRUCT)
kfree(chip->badblock_pattern);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(nand_release);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index fbe8e16..c15b1c4 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -39,7 +39,7 @@ extern int nand_scan_ident(struct mtd_info *mtd, int max_chips,
extern int nand_scan_tail(struct mtd_info *mtd);
/* Free resources held by the NAND device */
-extern void nand_release(struct mtd_info *mtd);
+extern int nand_release(struct mtd_info *mtd);
/* Internal helper for board drivers which need to override command function */
extern void nand_wait_ready(struct mtd_info *mtd);
Thanks,
//richard
next reply other threads:[~2016-07-03 13:39 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-03 13:38 Richard Weinberger [this message]
2016-07-04 9:16 ` Race-free NAND device removal Boris Brezillon
2016-07-04 9:44 ` Richard Weinberger
2016-07-04 10:06 ` Boris Brezillon
2016-07-04 11:02 ` Richard Weinberger
2016-07-04 11:11 ` Richard Weinberger
2016-07-04 12:02 ` Boris Brezillon
2016-07-04 21:34 ` Richard Weinberger
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=57791562.2020703@nod.at \
--to=richard@nod.at \
--cc=boris.brezillon@free-electrons.com \
--cc=computersforpeace@gmail.com \
--cc=linux-mtd@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.