2020-09-21 07:22:25

by Christoph Hellwig

[permalink] [raw]
Subject: remove blkdev_get as a public API v2

Hi Jens,

this series removes blkdev_get as a public API, leaving it as just an
implementation detail of blkdev_get_by_path and blkdev_get_by_dev. The
reason for that is that blkdev_get is a very confusing API that requires
a struct block_device to be fed in, but then actually consumes the
reference. And it turns out just using the two above mentioned APIs
actually significantly simplifies the code as well.

Changes since v1:
- fix a mismerged that left a stray bdget_disk around
- factour the partition scan at registration time code into a new
helper.

Diffstat:
block/genhd.c | 35 ++++++---------
block/ioctl.c | 13 ++---
drivers/block/nbd.c | 8 +--
drivers/block/pktcdvd.c | 92 +++++-----------------------------------
drivers/block/zram/zram_drv.c | 7 +--
drivers/char/raw.c | 51 ++++++++--------------
drivers/ide/ide-gd.c | 2
drivers/s390/block/dasd_genhd.c | 15 +-----
fs/block_dev.c | 12 ++---
fs/ocfs2/cluster/heartbeat.c | 28 ++++--------
include/linux/blk_types.h | 4 -
include/linux/blkdev.h | 1
include/linux/genhd.h | 2
include/linux/suspend.h | 4 -
include/linux/swap.h | 3 -
kernel/power/swap.c | 21 +++------
kernel/power/user.c | 26 +++--------
mm/swapfile.c | 45 ++++++++++---------
18 files changed, 130 insertions(+), 239 deletions(-)


2020-09-21 07:23:10

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 14/14] block: mark blkdev_get static

There are no users outside the core block code left now.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/block_dev.c | 3 +--
include/linux/blkdev.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 2898d69be6b3e4..6b9d19ffa5af7b 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1616,7 +1616,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, void *holder,
* RETURNS:
* 0 on success, -errno on failure.
*/
-int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
+static int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
{
int ret, perm = 0;

@@ -1637,7 +1637,6 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
bdput(bdev);
return ret;
}
-EXPORT_SYMBOL(blkdev_get);

/**
* blkdev_get_by_path - open a block device by name
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 5bd96fbab9b4c8..14117995091224 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1975,7 +1975,6 @@ void blkdev_show(struct seq_file *seqf, off_t offset);
#define BLKDEV_MAJOR_MAX 0
#endif

-int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder);
struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
void *holder);
struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder);
--
2.28.0

2020-09-21 07:23:23

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 04/14] pktcdvd: remove the if 0'ed pkt_start_recovery function

Remove code which has been dead since the initial commit.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/pktcdvd.c | 67 ++---------------------------------------
1 file changed, 2 insertions(+), 65 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 17f2e6ff122314..bc870a5f15f77b 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1082,65 +1082,6 @@ static void pkt_put_packet_data(struct pktcdvd_device *pd, struct packet_data *p
}
}

-/*
- * recover a failed write, query for relocation if possible
- *
- * returns 1 if recovery is possible, or 0 if not
- *
- */
-static int pkt_start_recovery(struct packet_data *pkt)
-{
- /*
- * FIXME. We need help from the file system to implement
- * recovery handling.
- */
- return 0;
-#if 0
- struct request *rq = pkt->rq;
- struct pktcdvd_device *pd = rq->rq_disk->private_data;
- struct block_device *pkt_bdev;
- struct super_block *sb = NULL;
- unsigned long old_block, new_block;
- sector_t new_sector;
-
- pkt_bdev = bdget(kdev_t_to_nr(pd->pkt_dev));
- if (pkt_bdev) {
- sb = get_super(pkt_bdev);
- bdput(pkt_bdev);
- }
-
- if (!sb)
- return 0;
-
- if (!sb->s_op->relocate_blocks)
- goto out;
-
- old_block = pkt->sector / (CD_FRAMESIZE >> 9);
- if (sb->s_op->relocate_blocks(sb, old_block, &new_block))
- goto out;
-
- new_sector = new_block * (CD_FRAMESIZE >> 9);
- pkt->sector = new_sector;
-
- bio_reset(pkt->bio);
- bio_set_dev(pkt->bio, pd->bdev);
- bio_set_op_attrs(pkt->bio, REQ_OP_WRITE, 0);
- pkt->bio->bi_iter.bi_sector = new_sector;
- pkt->bio->bi_iter.bi_size = pkt->frames * CD_FRAMESIZE;
- pkt->bio->bi_vcnt = pkt->frames;
-
- pkt->bio->bi_end_io = pkt_end_io_packet_write;
- pkt->bio->bi_private = pkt;
-
- drop_super(sb);
- return 1;
-
-out:
- drop_super(sb);
- return 0;
-#endif
-}
-
static inline void pkt_set_state(struct packet_data *pkt, enum packet_data_state state)
{
#if PACKET_DEBUG > 1
@@ -1357,12 +1298,8 @@ static void pkt_run_state_machine(struct pktcdvd_device *pd, struct packet_data
break;

case PACKET_RECOVERY_STATE:
- if (pkt_start_recovery(pkt)) {
- pkt_start_write(pd, pkt);
- } else {
- pkt_dbg(2, pd, "No recovery possible\n");
- pkt_set_state(pkt, PACKET_FINISHED_STATE);
- }
+ pkt_dbg(2, pd, "No recovery possible\n");
+ pkt_set_state(pkt, PACKET_FINISHED_STATE);
break;

case PACKET_FINISHED_STATE:
--
2.28.0

2020-09-21 07:23:32

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 07/14] raw: don't keep unopened block device around

Turn binding into a normal dev_t as the struct block device doesn't
buy us anything and use blkdev_open_by_dev to actually open it.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/char/raw.c | 51 +++++++++++++++++-----------------------------
1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/char/raw.c b/drivers/char/raw.c
index ccf5bd528642da..5d52a1f4738c76 100644
--- a/drivers/char/raw.c
+++ b/drivers/char/raw.c
@@ -28,7 +28,8 @@
#include <linux/uaccess.h>

struct raw_device_data {
- struct block_device *binding;
+ dev_t binding;
+ struct block_device *bdev;
int inuse;
};

@@ -73,14 +74,15 @@ static int raw_open(struct inode *inode, struct file *filp)
/*
* All we need to do on open is check that the device is bound.
*/
- bdev = raw_devices[minor].binding;
err = -ENODEV;
- if (!bdev)
+ if (!raw_devices[minor].binding)
goto out;
- bdgrab(bdev);
- err = blkdev_get(bdev, filp->f_mode | FMODE_EXCL, raw_open);
- if (err)
+ bdev = blkdev_get_by_dev(raw_devices[minor].binding,
+ filp->f_mode | FMODE_EXCL, raw_open);
+ if (IS_ERR(bdev)) {
+ err = PTR_ERR(bdev);
goto out;
+ }
err = set_blocksize(bdev, bdev_logical_block_size(bdev));
if (err)
goto out1;
@@ -90,6 +92,7 @@ static int raw_open(struct inode *inode, struct file *filp)
file_inode(filp)->i_mapping =
bdev->bd_inode->i_mapping;
filp->private_data = bdev;
+ raw_devices[minor].bdev = bdev;
mutex_unlock(&raw_mutex);
return 0;

@@ -110,7 +113,7 @@ static int raw_release(struct inode *inode, struct file *filp)
struct block_device *bdev;

mutex_lock(&raw_mutex);
- bdev = raw_devices[minor].binding;
+ bdev = raw_devices[minor].bdev;
if (--raw_devices[minor].inuse == 0)
/* Here inode->i_mapping == bdev->bd_inode->i_mapping */
inode->i_mapping = &inode->i_data;
@@ -133,6 +136,7 @@ raw_ioctl(struct file *filp, unsigned int command, unsigned long arg)
static int bind_set(int number, u64 major, u64 minor)
{
dev_t dev = MKDEV(major, minor);
+ dev_t raw = MKDEV(RAW_MAJOR, number);
struct raw_device_data *rawdev;
int err = 0;

@@ -166,25 +170,17 @@ static int bind_set(int number, u64 major, u64 minor)
mutex_unlock(&raw_mutex);
return -EBUSY;
}
- if (rawdev->binding) {
- bdput(rawdev->binding);
+ if (rawdev->binding)
module_put(THIS_MODULE);
- }
+
+ rawdev->binding = dev;
if (!dev) {
/* unbind */
- rawdev->binding = NULL;
- device_destroy(raw_class, MKDEV(RAW_MAJOR, number));
+ device_destroy(raw_class, raw);
} else {
- rawdev->binding = bdget(dev);
- if (rawdev->binding == NULL) {
- err = -ENOMEM;
- } else {
- dev_t raw = MKDEV(RAW_MAJOR, number);
- __module_get(THIS_MODULE);
- device_destroy(raw_class, raw);
- device_create(raw_class, NULL, raw, NULL,
- "raw%d", number);
- }
+ __module_get(THIS_MODULE);
+ device_destroy(raw_class, raw);
+ device_create(raw_class, NULL, raw, NULL, "raw%d", number);
}
mutex_unlock(&raw_mutex);
return err;
@@ -192,18 +188,9 @@ static int bind_set(int number, u64 major, u64 minor)

static int bind_get(int number, dev_t *dev)
{
- struct raw_device_data *rawdev;
- struct block_device *bdev;
-
if (number <= 0 || number >= max_raw_minors)
return -EINVAL;
-
- rawdev = &raw_devices[number];
-
- mutex_lock(&raw_mutex);
- bdev = rawdev->binding;
- *dev = bdev ? bdev->bd_dev : 0;
- mutex_unlock(&raw_mutex);
+ *dev = raw_devices[number].binding;
return 0;
}

--
2.28.0

2020-09-21 07:23:35

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 05/14] pktcdvd: use blkdev_get_by_dev instead of open coding it

Replace bdget + blkdev_get by blkdev_get_by_dev.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/pktcdvd.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index bc870a5f15f77b..467dbd06b7cdb1 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2110,16 +2110,18 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
int ret;
long lba;
struct request_queue *q;
+ struct block_device *bdev;

/*
* We need to re-open the cdrom device without O_NONBLOCK to be able
* to read/write from/to it. It is already opened in O_NONBLOCK mode
- * so bdget() can't fail.
+ * so open should not fail.
*/
- bdget(pd->bdev->bd_dev);
- ret = blkdev_get(pd->bdev, FMODE_READ | FMODE_EXCL, pd);
- if (ret)
+ bdev = blkdev_get_by_dev(pd->bdev->bd_dev, FMODE_READ | FMODE_EXCL, pd);
+ if (IS_ERR(bdev)) {
+ ret = PTR_ERR(bdev);
goto out;
+ }

ret = pkt_get_last_written(pd, &lba);
if (ret) {
@@ -2163,7 +2165,7 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
return 0;

out_putdev:
- blkdev_put(pd->bdev, FMODE_READ | FMODE_EXCL);
+ blkdev_put(bdev, FMODE_READ | FMODE_EXCL);
out:
return ret;
}
@@ -2500,7 +2502,6 @@ static int pkt_seq_show(struct seq_file *m, void *p)
static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
{
int i;
- int ret = 0;
char b[BDEVNAME_SIZE];
struct block_device *bdev;

@@ -2523,12 +2524,9 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
}
}

- bdev = bdget(dev);
- if (!bdev)
- return -ENOMEM;
- ret = blkdev_get(bdev, FMODE_READ | FMODE_NDELAY, NULL);
- if (ret)
- return ret;
+ bdev = blkdev_get_by_dev(dev, FMODE_READ | FMODE_NDELAY, NULL);
+ if (IS_ERR(bdev))
+ return PTR_ERR(bdev);
if (!blk_queue_scsi_passthrough(bdev_get_queue(bdev))) {
blkdev_put(bdev, FMODE_READ | FMODE_NDELAY);
return -EINVAL;
@@ -2546,7 +2544,6 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
if (IS_ERR(pd->cdrw.thread)) {
pkt_err(pd, "can't start kernel thread\n");
- ret = -ENOMEM;
goto out_mem;
}

@@ -2558,7 +2555,7 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
blkdev_put(bdev, FMODE_READ | FMODE_NDELAY);
/* This is safe: open() is still holding a reference. */
module_put(THIS_MODULE);
- return ret;
+ return -ENOMEM;
}

static int pkt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg)
--
2.28.0

2020-09-21 07:23:57

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 11/14] PM: rewrite is_hibernate_resume_dev to not require an inode

Just check the dev_t to help simplifying the code.

Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
---
fs/block_dev.c | 2 +-
include/linux/suspend.h | 4 ++--
kernel/power/user.c | 12 ++++++------
3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 1a9325f4315769..2898d69be6b3e4 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1885,7 +1885,7 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (bdev_read_only(I_BDEV(bd_inode)))
return -EPERM;

- if (IS_SWAPFILE(bd_inode) && !is_hibernate_resume_dev(bd_inode))
+ if (IS_SWAPFILE(bd_inode) && !is_hibernate_resume_dev(bd_inode->i_rdev))
return -ETXTBSY;

if (!iov_iter_count(from))
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index cb9afad82a90c8..8af13ba60c7e45 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -473,9 +473,9 @@ static inline int hibernate_quiet_exec(int (*func)(void *data), void *data) {
#endif /* CONFIG_HIBERNATION */

#ifdef CONFIG_HIBERNATION_SNAPSHOT_DEV
-int is_hibernate_resume_dev(const struct inode *);
+int is_hibernate_resume_dev(dev_t dev);
#else
-static inline int is_hibernate_resume_dev(const struct inode *i) { return 0; }
+static inline int is_hibernate_resume_dev(dev_t dev) { return 0; }
#endif

/* Hibernation and suspend events */
diff --git a/kernel/power/user.c b/kernel/power/user.c
index d5eedc2baa2a10..b5815685b944fe 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -35,12 +35,12 @@ static struct snapshot_data {
bool ready;
bool platform_support;
bool free_bitmaps;
- struct inode *bd_inode;
+ dev_t dev;
} snapshot_state;

-int is_hibernate_resume_dev(const struct inode *bd_inode)
+int is_hibernate_resume_dev(dev_t dev)
{
- return hibernation_available() && snapshot_state.bd_inode == bd_inode;
+ return hibernation_available() && snapshot_state.dev == dev;
}

static int snapshot_open(struct inode *inode, struct file *filp)
@@ -101,7 +101,7 @@ static int snapshot_open(struct inode *inode, struct file *filp)
data->frozen = false;
data->ready = false;
data->platform_support = false;
- data->bd_inode = NULL;
+ data->dev = 0;

Unlock:
unlock_system_sleep();
@@ -117,7 +117,7 @@ static int snapshot_release(struct inode *inode, struct file *filp)

swsusp_free();
data = filp->private_data;
- data->bd_inode = NULL;
+ data->dev = 0;
free_all_swap_pages(data->swap);
if (data->frozen) {
pm_restore_gfp_mask();
@@ -245,7 +245,7 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
if (data->swap < 0)
return -ENODEV;

- data->bd_inode = bdev->bd_inode;
+ data->dev = bdev->bd_dev;
bdput(bdev);
return 0;
}
--
2.28.0

2020-09-21 07:23:57

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 08/14] dasd: cleanup dasd_scan_partitions

Use blkdev_get_by_dev instead of bdget_disk + blkdev_get.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/s390/block/dasd_genhd.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
index af5b0ecb8f8923..a9698fba9b76ce 100644
--- a/drivers/s390/block/dasd_genhd.c
+++ b/drivers/s390/block/dasd_genhd.c
@@ -101,18 +101,11 @@ int dasd_scan_partitions(struct dasd_block *block)
struct block_device *bdev;
int rc;

- bdev = bdget_disk(block->gdp, 0);
- if (!bdev) {
- DBF_DEV_EVENT(DBF_ERR, block->base, "%s",
- "scan partitions error, bdget returned NULL");
- return -ENODEV;
- }
-
- rc = blkdev_get(bdev, FMODE_READ, NULL);
- if (rc < 0) {
+ bdev = blkdev_get_by_dev(disk_devt(block->gdp), FMODE_READ, NULL);
+ if (IS_ERR(bdev)) {
DBF_DEV_EVENT(DBF_ERR, block->base,
- "scan partitions error, blkdev_get returned %d",
- rc);
+ "scan partitions error, blkdev_get returned %ld",
+ PTR_ERR(bdev));
return -ENODEV;
}

--
2.28.0

2020-09-21 07:24:33

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 13/14] PM: mm: cleanup swsusp_swap_check

Use blkdev_get_by_dev instead of bdget + blkdev_get.

Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/swap.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 9d3ffbfe08dbf6..71385bedcc3a49 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -343,12 +343,10 @@ static int swsusp_swap_check(void)
return res;
root_swap = res;

- hib_resume_bdev = bdget(swsusp_resume_device);
- if (!hib_resume_bdev)
- return -ENOMEM;
- res = blkdev_get(hib_resume_bdev, FMODE_WRITE, NULL);
- if (res)
- return res;
+ hib_resume_bdev = blkdev_get_by_dev(swsusp_resume_device, FMODE_WRITE,
+ NULL);
+ if (IS_ERR(hib_resume_bdev))
+ return PTR_ERR(hib_resume_bdev);

res = set_blocksize(hib_resume_bdev, PAGE_SIZE);
if (res < 0)
--
2.28.0

2020-09-21 07:25:18

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 06/14] zram: cleanup backing_dev_store

Use blkdev_get_by_dev instead of bdgrab + blkdev_get.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/zram/zram_drv.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index a356275605b104..91ccfe444525b4 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -491,9 +491,10 @@ static ssize_t backing_dev_store(struct device *dev,
goto out;
}

- bdev = bdgrab(I_BDEV(inode));
- err = blkdev_get(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL, zram);
- if (err < 0) {
+ bdev = blkdev_get_by_dev(inode->i_rdev,
+ FMODE_READ | FMODE_WRITE | FMODE_EXCL, zram);
+ if (IS_ERR(bdev)) {
+ err = PTR_ERR(bdev);
bdev = NULL;
goto out;
}
--
2.28.0

2020-09-21 08:47:18

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 08/14] dasd: cleanup dasd_scan_partitions

Hello!

On 21.09.2020 10:19, Christoph Hellwig wrote:

> Use blkdev_get_by_dev instead of bdget_disk + blkdev_get.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/s390/block/dasd_genhd.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
> index af5b0ecb8f8923..a9698fba9b76ce 100644
> --- a/drivers/s390/block/dasd_genhd.c
> +++ b/drivers/s390/block/dasd_genhd.c
> @@ -101,18 +101,11 @@ int dasd_scan_partitions(struct dasd_block *block)
> struct block_device *bdev;
> int rc;
>
> - bdev = bdget_disk(block->gdp, 0);
> - if (!bdev) {
> - DBF_DEV_EVENT(DBF_ERR, block->base, "%s",
> - "scan partitions error, bdget returned NULL");
> - return -ENODEV;
> - }
> -
> - rc = blkdev_get(bdev, FMODE_READ, NULL);
> - if (rc < 0) {
> + bdev = blkdev_get_by_dev(disk_devt(block->gdp), FMODE_READ, NULL);
> + if (IS_ERR(bdev)) {
> DBF_DEV_EVENT(DBF_ERR, block->base,
> - "scan partitions error, blkdev_get returned %d",
> - rc);
> + "scan partitions error, blkdev_get returned %ld",

blkdev_get_by_dev() now?

> + PTR_ERR(bdev));
> return -ENODEV;
> }
>

MBR, Sergei

2020-09-21 21:48:00

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 11/14] PM: rewrite is_hibernate_resume_dev to not require an inode

On Mon 2020-09-21 09:19:55, Christoph Hellwig wrote:
> Just check the dev_t to help simplifying the code.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> Acked-by: Rafael J. Wysocki <[email protected]>

Acked-by: Pavel Machek <[email protected]>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (403.00 B)
signature.asc (188.00 B)
Digital signature
Download all attachments

2020-09-22 15:22:02

by Stefan Haberland

[permalink] [raw]
Subject: Re: [PATCH 08/14] dasd: cleanup dasd_scan_partitions

Am 21.09.20 um 09:19 schrieb Christoph Hellwig:
> Use blkdev_get_by_dev instead of bdget_disk + blkdev_get.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Beside what Sergei mentioned...

Reviewed-by: Stefan Haberland <[email protected]>


> ---
> drivers/s390/block/dasd_genhd.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
> index af5b0ecb8f8923..a9698fba9b76ce 100644
> --- a/drivers/s390/block/dasd_genhd.c
> +++ b/drivers/s390/block/dasd_genhd.c
> @@ -101,18 +101,11 @@ int dasd_scan_partitions(struct dasd_block *block)
> struct block_device *bdev;
> int rc;
>
> - bdev = bdget_disk(block->gdp, 0);
> - if (!bdev) {
> - DBF_DEV_EVENT(DBF_ERR, block->base, "%s",
> - "scan partitions error, bdget returned NULL");
> - return -ENODEV;
> - }
> -
> - rc = blkdev_get(bdev, FMODE_READ, NULL);
> - if (rc < 0) {
> + bdev = blkdev_get_by_dev(disk_devt(block->gdp), FMODE_READ, NULL);
> + if (IS_ERR(bdev)) {
> DBF_DEV_EVENT(DBF_ERR, block->base,
> - "scan partitions error, blkdev_get returned %d",
> - rc);
> + "scan partitions error, blkdev_get returned %ld",
> + PTR_ERR(bdev));
> return -ENODEV;
> }
>

2020-09-23 16:45:22

by Jens Axboe

[permalink] [raw]
Subject: Re: remove blkdev_get as a public API v2

On 9/21/20 1:19 AM, Christoph Hellwig wrote:
> Hi Jens,
>
> this series removes blkdev_get as a public API, leaving it as just an
> implementation detail of blkdev_get_by_path and blkdev_get_by_dev. The
> reason for that is that blkdev_get is a very confusing API that requires
> a struct block_device to be fed in, but then actually consumes the
> reference. And it turns out just using the two above mentioned APIs
> actually significantly simplifies the code as well.
>
> Changes since v1:
> - fix a mismerged that left a stray bdget_disk around
> - factour the partition scan at registration time code into a new
> helper.

Applied for 5.10, thanks.

--
Jens Axboe