There are two bdev exclusive open bugs.

* open_bdev_exclusive() must not be called with NULL holder. Use dev
as the holder.

* open_by_devnum() doesn't open the bdev exclusively but
block2mtd_free_device() always assumes it. Explicitly claim the
bdev.

The latter is rather clumsy but will be simplified with future
blkdev_get/put() cleanups.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: linux-mtd@lists.infradead.org
---
drivers/mtd/devices/block2mtd.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 2cf0cc6..806a9ed 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -246,7 +246,7 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
return NULL;

/* Get a handle on the device */
- bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, NULL);
+ bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, dev);
#ifndef MODULE
if (IS_ERR(bdev)) {

@@ -256,6 +256,15 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
dev_t devt = name_to_dev_t(devname);
if (devt) {
bdev = open_by_devnum(devt, FMODE_WRITE | FMODE_READ);
+ if (!IS_ERR(bdev)) {
+ int ret;
+ ret = bd_claim(bdev, dev);
+ if (ret) {
+ blkdev_put(bdev,
+ FMODE_READ | FMODE_WRITE);
+ bdev = ERR_PTR(ret);
+ }
+ }
}
}
#endif
--
1.7.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Hello,

On 11/13/2010 11:38 AM, Artem Bityutskiy wrote:
> On Mon, 2010-11-01 at 17:15 +0100, Tejun Heo wrote:
>> + if (!IS_ERR(bdev)) {
>> + int ret;
>> + ret = bd_claim(bdev, dev);
>> + if (ret) {
>> + blkdev_put(bdev,
>> + FMODE_READ | FMODE_WRITE);
>
> Would be a bit cleaner to define ea temporary variable:
>
> fmode_t mode = FMODE_READ | FMODE_WRITE;
>
> Would you to re-send with this little change please?

Yeap, sure.

> And ideally, 2 independent patches would be nicer because you fix 2
> independent issues.

Hmmm... not really. The patch is small enough and splitting it won't
really buy as any better bisectability. Splitting patches into
logical changes is a good thing but it's no religious dogma. Let's
apply it to the point it actually helps.

Thank you.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

There are two bdev exclusive open bugs.

* open_bdev_exclusive() must not be called with NULL holder. Use dev
as the holder.

* open_by_devnum() doesn't open the bdev exclusively but
block2mtd_free_device() always assumes it. Explicitly claim the
bdev.

The latter is rather clumsy but will be simplified with future
blkdev_get/put() cleanups.

- Updated to use local variable @mode to cache FMODE_* masks as
suggested by Artem Bityutskiy.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <dedekind1@gmail.com>
---

This change will cause two conflicts in the series. I'm not reposting
them now as they're mostly trivial. Git tree is fully updated.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git cleanup-bd_claim

Jens, please let me know when/if you want the full series reposted.

Thanks.

drivers/mtd/devices/block2mtd.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

Index: work/drivers/mtd/devices/block2mtd.c
================================================== =================
--- work.orig/drivers/mtd/devices/block2mtd.c
+++ work/drivers/mtd/devices/block2mtd.c
@@ -234,6 +234,7 @@ static void block2mtd_free_device(struct
/* FIXME: ensure that mtd->size % erase_size == 0 */
static struct block2mtd_dev *add_device(char *devname, int erase_size)
{
+ const fmode_t mode = FMODE_READ | FMODE_WRITE;
struct block_device *bdev;
struct block2mtd_dev *dev;
char *name;
@@ -246,7 +247,7 @@ static struct block2mtd_dev *add_device(
return NULL;

/* Get a handle on the device */
- bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, NULL);
+ bdev = open_bdev_exclusive(devname, mode, dev);
#ifndef MODULE
if (IS_ERR(bdev)) {

@@ -255,7 +256,15 @@ static struct block2mtd_dev *add_device(

dev_t devt = name_to_dev_t(devname);
if (devt) {
- bdev = open_by_devnum(devt, FMODE_WRITE | FMODE_READ);
+ bdev = open_by_devnum(devt, mode);
+ if (!IS_ERR(bdev)) {
+ int ret;
+ ret = bd_claim(bdev, dev);
+ if (ret) {
+ blkdev_put(bdev, mode);
+ bdev = ERR_PTR(ret);
+ }
+ }
}
}
#endif

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel