2020-09-17 17:04:09

by Christoph Hellwig

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

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.

Diffstat:
block/genhd.c | 11 ++--
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 | 13 +----
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, 119 insertions(+), 224 deletions(-)


2020-09-17 17:06:34

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 01/14] block: move the NEED_PART_SCAN flag to struct gendisk

We can only scan for partitions on the whole disk, so move the flag
from struct block_device to struct gendisk.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/genhd.c | 4 ++--
drivers/block/nbd.c | 8 ++++----
drivers/ide/ide-gd.c | 2 +-
fs/block_dev.c | 7 +++----
include/linux/blk_types.h | 4 +---
include/linux/genhd.h | 2 ++
6 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 9d060e79eb31d8..7b56203c90a303 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -731,7 +731,7 @@ static void register_disk(struct device *parent, struct gendisk *disk,
if (!bdev)
goto exit;

- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ set_bit(GD_NEED_PART_SCAN, &disk->state);
err = blkdev_get(bdev, FMODE_READ, NULL);
if (err < 0)
goto exit;
@@ -2112,7 +2112,7 @@ bool bdev_check_media_change(struct block_device *bdev)
if (__invalidate_device(bdev, true))
pr_warn("VFS: busy inodes on changed media %s\n",
bdev->bd_disk->disk_name);
- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
return true;
}
EXPORT_SYMBOL(bdev_check_media_change);
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 15eed210feeff4..2dca0aab0a9a25 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -315,7 +315,7 @@ static void nbd_size_update(struct nbd_device *nbd)
bd_set_nr_sectors(bdev, nr_sectors);
set_blocksize(bdev, config->blksize);
} else
- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ set_bit(GD_NEED_PART_SCAN, &nbd->disk->state);
bdput(bdev);
}
kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
@@ -1322,7 +1322,7 @@ static int nbd_start_device_ioctl(struct nbd_device *nbd, struct block_device *b
return ret;

if (max_part)
- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ set_bit(GD_NEED_PART_SCAN, &nbd->disk->state);
mutex_unlock(&nbd->config_lock);
ret = wait_event_interruptible(config->recv_wq,
atomic_read(&config->recv_threads) == 0);
@@ -1500,9 +1500,9 @@ static int nbd_open(struct block_device *bdev, fmode_t mode)
refcount_set(&nbd->config_refs, 1);
refcount_inc(&nbd->refs);
mutex_unlock(&nbd->config_lock);
- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
} else if (nbd_disconnected(nbd->config)) {
- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
}
out:
mutex_unlock(&nbd_index_mutex);
diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
index 661e2aa9c96784..e2b6c82586ce8b 100644
--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -230,7 +230,7 @@ static int ide_gd_open(struct block_device *bdev, fmode_t mode)
bdev->bd_disk->disk_name);
drive->disk_ops->get_capacity(drive);
set_capacity(disk, ide_gd_capacity(drive));
- set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ set_bit(GD_NEED_PART_SCAN, &disk->state);
} else if (drive->dev_flags & IDE_DFLAG_FORMAT_IN_PROGRESS) {
ret = -EBUSY;
goto out_put_idkp;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 0b34955b9e360f..1a9325f4315769 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -910,7 +910,6 @@ struct block_device *bdget(dev_t dev)
bdev->bd_super = NULL;
bdev->bd_inode = inode;
bdev->bd_part_count = 0;
- bdev->bd_flags = 0;
inode->i_mode = S_IFBLK;
inode->i_rdev = dev;
inode->i_bdev = bdev;
@@ -1385,7 +1384,7 @@ int bdev_disk_changed(struct block_device *bdev, bool invalidate)

lockdep_assert_held(&bdev->bd_mutex);

- clear_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
+ clear_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);

rescan:
ret = blk_drop_partitions(bdev);
@@ -1509,7 +1508,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, void *holder,
* The latter is necessary to prevent ghost
* partitions on a removed medium.
*/
- if (test_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags) &&
+ if (test_bit(GD_NEED_PART_SCAN, &disk->state) &&
(!ret || ret == -ENOMEDIUM))
bdev_disk_changed(bdev, ret == -ENOMEDIUM);

@@ -1539,7 +1538,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, void *holder,
if (bdev->bd_disk->fops->open)
ret = bdev->bd_disk->fops->open(bdev, mode);
/* the same as first opener case, read comment there */
- if (test_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags) &&
+ if (test_bit(GD_NEED_PART_SCAN, &disk->state) &&
(!ret || ret == -ENOMEDIUM))
bdev_disk_changed(bdev, ret == -ENOMEDIUM);
if (ret)
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 6ffa783e16335e..eb20e28184ab19 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -19,8 +19,6 @@ struct cgroup_subsys_state;
typedef void (bio_end_io_t) (struct bio *);
struct bio_crypt_ctx;

-#define BDEV_NEED_PART_SCAN 0
-
struct block_device {
dev_t bd_dev;
int bd_openers;
@@ -39,7 +37,7 @@ struct block_device {
struct hd_struct * bd_part;
/* number of times partitions within this device have been opened. */
unsigned bd_part_count;
- unsigned long bd_flags;
+
spinlock_t bd_size_lock; /* for bd_inode->i_size updates */
struct gendisk * bd_disk;
struct backing_dev_info *bd_bdi;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 1c97cf84f011a7..38f23d75701379 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -191,6 +191,8 @@ struct gendisk {
void *private_data;

int flags;
+ unsigned long state;
+#define GD_NEED_PART_SCAN 0
struct rw_semaphore lookup_sem;
struct kobject *slave_dir;

--
2.28.0

2020-09-17 17:12:05

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 03/14] block: cleanup blkdev_bszset

Use blkdev_get_by_dev instead of bdgrab + blkdev_get.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/ioctl.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index ae74d0409afab9..06262c28f0c6c1 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -478,15 +478,14 @@ static int blkdev_bszset(struct block_device *bdev, fmode_t mode,
if (get_user(n, argp))
return -EFAULT;

- if (!(mode & FMODE_EXCL)) {
- bdgrab(bdev);
- if (blkdev_get(bdev, mode | FMODE_EXCL, &bdev) < 0)
- return -EBUSY;
- }
+ if (mode & FMODE_EXCL)
+ return set_blocksize(bdev, n);

+ if (IS_ERR(blkdev_get_by_dev(bdev->bd_dev, mode | FMODE_EXCL, &bdev)))
+ return -EBUSY;
ret = set_blocksize(bdev, n);
- if (!(mode & FMODE_EXCL))
- blkdev_put(bdev, mode | FMODE_EXCL);
+ blkdev_put(bdev, mode | FMODE_EXCL);
+
return ret;
}

--
2.28.0

2020-09-17 17:12:52

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-17 17:15:29

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-17 17:17:10

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-17 17:18:32

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-17 17:20:06

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 | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
index af5b0ecb8f8923..70c09a46d22806 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);
+ IS_ERR(bdev));
return -ENODEV;
}

--
2.28.0

2020-09-17 17:21:44

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 09/14] ocfs2: cleanup o2hb_region_dev_store

Use blkdev_get_by_dev instead of igrab (aka open coded bdgrab) +
blkdev_get.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ocfs2/cluster/heartbeat.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 89d13e0705fe7b..0179a73a3fa2c4 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -1766,7 +1766,6 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
int sectsize;
char *p = (char *)page;
struct fd f;
- struct inode *inode;
ssize_t ret = -EINVAL;
int live_threshold;

@@ -1793,20 +1792,16 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
reg->hr_block_bytes == 0)
goto out2;

- inode = igrab(f.file->f_mapping->host);
- if (inode == NULL)
+ if (!S_ISBLK(f.file->f_mapping->host->i_mode))
goto out2;

- if (!S_ISBLK(inode->i_mode))
- goto out3;
-
- reg->hr_bdev = I_BDEV(f.file->f_mapping->host);
- ret = blkdev_get(reg->hr_bdev, FMODE_WRITE | FMODE_READ, NULL);
- if (ret) {
+ reg->hr_bdev = blkdev_get_by_dev(f.file->f_mapping->host->i_rdev,
+ FMODE_WRITE | FMODE_READ, NULL);
+ if (IS_ERR(reg->hr_bdev)) {
+ ret = PTR_ERR(reg->hr_bdev);
reg->hr_bdev = NULL;
- goto out3;
+ goto out2;
}
- inode = NULL;

bdevname(reg->hr_bdev, reg->hr_dev_name);

@@ -1909,16 +1904,13 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
config_item_name(&reg->hr_item), reg->hr_dev_name);

out3:
- iput(inode);
+ if (ret < 0) {
+ blkdev_put(reg->hr_bdev, FMODE_READ | FMODE_WRITE);
+ reg->hr_bdev = NULL;
+ }
out2:
fdput(f);
out:
- if (ret < 0) {
- if (reg->hr_bdev) {
- blkdev_put(reg->hr_bdev, FMODE_READ|FMODE_WRITE);
- reg->hr_bdev = NULL;
- }
- }
return ret;
}

--
2.28.0

2020-09-17 17:24:10

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 10/14] mm: cleanup claim_swapfile

Use blkdev_get_by_dev instead of bdgrab + blkdev_get.

Signed-off-by: Christoph Hellwig <[email protected]>
---
mm/swapfile.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 12f59e641b5e29..7438c4affc75fa 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2920,10 +2920,10 @@ static int claim_swapfile(struct swap_info_struct *p, struct inode *inode)
int error;

if (S_ISBLK(inode->i_mode)) {
- p->bdev = bdgrab(I_BDEV(inode));
- error = blkdev_get(p->bdev,
+ p->bdev = blkdev_get_by_dev(inode->i_rdev,
FMODE_READ | FMODE_WRITE | FMODE_EXCL, p);
- if (error < 0) {
+ if (IS_ERR(p->bdev)) {
+ error = PTR_ERR(p->bdev);
p->bdev = NULL;
return error;
}
--
2.28.0

2020-09-17 17:25:27

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]>
---
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-17 17:28:56

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 12/14] mm: split swap_type_of

swap_type_of is used for two entirely different purposes:

(1) check what swap type a given device/offset corresponds to
(2) find the first available swap device that can be written to

Mixing both in a single function creates an unreadable mess. Create two
separate functions instead, and switch both to pass a dev_t instead of
a struct block_device to further simplify the code.

Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/swap.h | 3 ++-
kernel/power/swap.c | 17 ++++++++---------
kernel/power/user.c | 16 ++++------------
mm/swapfile.c | 39 +++++++++++++++++++++------------------
4 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 661046994db4b9..4340a7b6e7a1dc 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -467,7 +467,8 @@ extern int swapcache_prepare(swp_entry_t);
extern void swap_free(swp_entry_t);
extern void swapcache_free_entries(swp_entry_t *entries, int n);
extern int free_swap_and_cache(swp_entry_t);
-extern int swap_type_of(dev_t, sector_t, struct block_device **);
+int swap_type_of(dev_t device, sector_t offset);
+int find_first_swap(dev_t *device);
extern unsigned int count_swap_pages(int, int);
extern sector_t map_swap_page(struct page *, struct block_device **);
extern sector_t swapdev_block(int, pgoff_t);
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 01e2858b5fe367..9d3ffbfe08dbf6 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -335,12 +335,17 @@ static int swsusp_swap_check(void)
{
int res;

- res = swap_type_of(swsusp_resume_device, swsusp_resume_block,
- &hib_resume_bdev);
+ if (swsusp_resume_device)
+ res = swap_type_of(swsusp_resume_device, swsusp_resume_block);
+ else
+ res = find_first_swap(&swsusp_resume_device);
if (res < 0)
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;
@@ -349,12 +354,6 @@ static int swsusp_swap_check(void)
if (res < 0)
blkdev_put(hib_resume_bdev, FMODE_WRITE);

- /*
- * Update the resume device to the one actually used,
- * so the test_resume mode can use it in case it is
- * invoked from hibernate() to test the snapshot.
- */
- swsusp_resume_device = hib_resume_bdev->bd_dev;
return res;
}

diff --git a/kernel/power/user.c b/kernel/power/user.c
index b5815685b944fe..6510a8f7687f42 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -69,8 +69,7 @@ static int snapshot_open(struct inode *inode, struct file *filp)
memset(&data->handle, 0, sizeof(struct snapshot_handle));
if ((filp->f_flags & O_ACCMODE) == O_RDONLY) {
/* Hibernating. The image device should be accessible. */
- data->swap = swsusp_resume_device ?
- swap_type_of(swsusp_resume_device, 0, NULL) : -1;
+ data->swap = swap_type_of(swsusp_resume_device, 0);
data->mode = O_RDONLY;
data->free_bitmaps = false;
error = __pm_notifier_call_chain(PM_HIBERNATION_PREPARE, -1, &nr_calls);
@@ -210,7 +209,6 @@ struct compat_resume_swap_area {
static int snapshot_set_swap_area(struct snapshot_data *data,
void __user *argp)
{
- struct block_device *bdev;
sector_t offset;
dev_t swdev;

@@ -237,16 +235,10 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
* User space encodes device types as two-byte values,
* so we need to recode them
*/
- if (!swdev) {
- data->swap = -1;
- return -EINVAL;
- }
- data->swap = swap_type_of(swdev, offset, &bdev);
+ data->swap = swap_type_of(swdev, offset);
if (data->swap < 0)
- return -ENODEV;
-
- data->dev = bdev->bd_dev;
- bdput(bdev);
+ return swdev ? -ENODEV : -EINVAL;
+ data->dev = swdev;
return 0;
}

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 7438c4affc75fa..b90f8692074397 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1801,13 +1801,12 @@ int free_swap_and_cache(swp_entry_t entry)
*
* This is needed for the suspend to disk (aka swsusp).
*/
-int swap_type_of(dev_t device, sector_t offset, struct block_device **bdev_p)
+int swap_type_of(dev_t device, sector_t offset)
{
- struct block_device *bdev = NULL;
int type;

- if (device)
- bdev = bdget(device);
+ if (!device)
+ return -1;

spin_lock(&swap_lock);
for (type = 0; type < nr_swapfiles; type++) {
@@ -1816,30 +1815,34 @@ int swap_type_of(dev_t device, sector_t offset, struct block_device **bdev_p)
if (!(sis->flags & SWP_WRITEOK))
continue;

- if (!bdev) {
- if (bdev_p)
- *bdev_p = bdgrab(sis->bdev);
-
- spin_unlock(&swap_lock);
- return type;
- }
- if (bdev == sis->bdev) {
+ if (device == sis->bdev->bd_dev) {
struct swap_extent *se = first_se(sis);

if (se->start_block == offset) {
- if (bdev_p)
- *bdev_p = bdgrab(sis->bdev);
-
spin_unlock(&swap_lock);
- bdput(bdev);
return type;
}
}
}
spin_unlock(&swap_lock);
- if (bdev)
- bdput(bdev);
+ return -ENODEV;
+}

+int find_first_swap(dev_t *device)
+{
+ int type;
+
+ spin_lock(&swap_lock);
+ for (type = 0; type < nr_swapfiles; type++) {
+ struct swap_info_struct *sis = swap_info[type];
+
+ if (!(sis->flags & SWP_WRITEOK))
+ continue;
+ *device = sis->bdev->bd_dev;
+ spin_unlock(&swap_lock);
+ return type;
+ }
+ spin_unlock(&swap_lock);
return -ENODEV;
}

--
2.28.0

2020-09-17 17:32:57

by Rafael J. Wysocki

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

On Thu, Sep 17, 2020 at 7:24 PM Christoph Hellwig <[email protected]> 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]>

> ---
> 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-17 17:36:28

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-17 18:13:36

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]>
---
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-17 18:42:20

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 02/14] block: switch register_disk to use blkdev_get_by_dev

Use blkdev_get_by_dev instead of open coding it using bdget_disk +
blkdev_get.

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

diff --git a/block/genhd.c b/block/genhd.c
index 7b56203c90a303..f778716fac6cde 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -732,10 +732,9 @@ static void register_disk(struct device *parent, struct gendisk *disk,
goto exit;

set_bit(GD_NEED_PART_SCAN, &disk->state);
- err = blkdev_get(bdev, FMODE_READ, NULL);
- if (err < 0)
- goto exit;
- blkdev_put(bdev, FMODE_READ);
+ bdev = blkdev_get_by_dev(disk_devt(disk), FMODE_READ, NULL);
+ if (!IS_ERR(bdev))
+ blkdev_put(bdev, FMODE_READ);

exit:
/* announce disk after possible partitions are created */
--
2.28.0

2020-09-18 02:46:03

by Joseph Qi

[permalink] [raw]
Subject: Re: [PATCH 09/14] ocfs2: cleanup o2hb_region_dev_store



On 2020/9/18 00:57, Christoph Hellwig wrote:
> Use blkdev_get_by_dev instead of igrab (aka open coded bdgrab) +
> blkdev_get.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Reviewed-by: Joseph Qi <[email protected]>

> ---
> fs/ocfs2/cluster/heartbeat.c | 28 ++++++++++------------------
> 1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
> index 89d13e0705fe7b..0179a73a3fa2c4 100644
> --- a/fs/ocfs2/cluster/heartbeat.c
> +++ b/fs/ocfs2/cluster/heartbeat.c
> @@ -1766,7 +1766,6 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
> int sectsize;
> char *p = (char *)page;
> struct fd f;
> - struct inode *inode;
> ssize_t ret = -EINVAL;
> int live_threshold;
>
> @@ -1793,20 +1792,16 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
> reg->hr_block_bytes == 0)
> goto out2;
>
> - inode = igrab(f.file->f_mapping->host);
> - if (inode == NULL)
> + if (!S_ISBLK(f.file->f_mapping->host->i_mode))
> goto out2;
>
> - if (!S_ISBLK(inode->i_mode))
> - goto out3;
> -
> - reg->hr_bdev = I_BDEV(f.file->f_mapping->host);
> - ret = blkdev_get(reg->hr_bdev, FMODE_WRITE | FMODE_READ, NULL);
> - if (ret) {
> + reg->hr_bdev = blkdev_get_by_dev(f.file->f_mapping->host->i_rdev,
> + FMODE_WRITE | FMODE_READ, NULL);
> + if (IS_ERR(reg->hr_bdev)) {
> + ret = PTR_ERR(reg->hr_bdev);
> reg->hr_bdev = NULL;
> - goto out3;
> + goto out2;
> }
> - inode = NULL;
>
> bdevname(reg->hr_bdev, reg->hr_dev_name);
>
> @@ -1909,16 +1904,13 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
> config_item_name(&reg->hr_item), reg->hr_dev_name);
>
> out3:
> - iput(inode);
> + if (ret < 0) {
> + blkdev_put(reg->hr_bdev, FMODE_READ | FMODE_WRITE);
> + reg->hr_bdev = NULL;
> + }
> out2:
> fdput(f);
> out:
> - if (ret < 0) {
> - if (reg->hr_bdev) {
> - blkdev_put(reg->hr_bdev, FMODE_READ|FMODE_WRITE);
> - reg->hr_bdev = NULL;
> - }
> - }
> return ret;
> }
>
>

2020-09-18 08:55:42

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 02/14] block: switch register_disk to use blkdev_get_by_dev

Hello!

On 17.09.2020 19:57, Christoph Hellwig wrote:

> Use blkdev_get_by_dev instead of open coding it using bdget_disk +
> blkdev_get.

I don't see where you are removing bdget_disk() call (situated just before
the below code?)...

> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> block/genhd.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/block/genhd.c b/block/genhd.c
> index 7b56203c90a303..f778716fac6cde 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -732,10 +732,9 @@ static void register_disk(struct device *parent, struct gendisk *disk,
> goto exit;
>
> set_bit(GD_NEED_PART_SCAN, &disk->state);
> - err = blkdev_get(bdev, FMODE_READ, NULL);
> - if (err < 0)
> - goto exit;
> - blkdev_put(bdev, FMODE_READ);
> + bdev = blkdev_get_by_dev(disk_devt(disk), FMODE_READ, NULL);
> + if (!IS_ERR(bdev))
> + blkdev_put(bdev, FMODE_READ);
>
> exit:
> /* announce disk after possible partitions are created */

MBR, Sergei

2020-09-18 16:05:52

by Rafael J. Wysocki

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

On Thu, Sep 17, 2020 at 7:39 PM Christoph Hellwig <[email protected]> wrote:
>
> 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-19 05:17:03

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 02/14] block: switch register_disk to use blkdev_get_by_dev

On Fri, Sep 18, 2020 at 11:52:39AM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 17.09.2020 19:57, Christoph Hellwig wrote:
>
>> Use blkdev_get_by_dev instead of open coding it using bdget_disk +
>> blkdev_get.
>
> I don't see where you are removing bdget_disk() call (situated just before
> the below code?)...

Indeed. That's what you get for a messy last minute rebase.. :(

2020-09-19 09:09:11

by Pavel Machek

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

On Thu 2020-09-17 18:57:19, Christoph Hellwig wrote:
> Use blkdev_get_by_dev instead of bdget + blkdev_get.
>
> Signed-off-by: Christoph Hellwig <[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) (345.00 B)
signature.asc (201.00 B)
Download all attachments

2020-09-19 09:10:01

by Pavel Machek

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

On Thu 2020-09-17 18:57:17, Christoph Hellwig wrote:
> Just check the dev_t to help simplifying the code.
>
> Signed-off-by: Christoph Hellwig <[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) (343.00 B)
signature.asc (201.00 B)
Download all attachments

2020-09-24 13:42:26

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 01/14] block: move the NEED_PART_SCAN flag to struct gendisk

On Thu 17-09-20 18:57:07, Christoph Hellwig wrote:
> We can only scan for partitions on the whole disk, so move the flag
> from struct block_device to struct gendisk.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Makes sense. You can add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> block/genhd.c | 4 ++--
> drivers/block/nbd.c | 8 ++++----
> drivers/ide/ide-gd.c | 2 +-
> fs/block_dev.c | 7 +++----
> include/linux/blk_types.h | 4 +---
> include/linux/genhd.h | 2 ++
> 6 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/block/genhd.c b/block/genhd.c
> index 9d060e79eb31d8..7b56203c90a303 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -731,7 +731,7 @@ static void register_disk(struct device *parent, struct gendisk *disk,
> if (!bdev)
> goto exit;
>
> - set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + set_bit(GD_NEED_PART_SCAN, &disk->state);
> err = blkdev_get(bdev, FMODE_READ, NULL);
> if (err < 0)
> goto exit;
> @@ -2112,7 +2112,7 @@ bool bdev_check_media_change(struct block_device *bdev)
> if (__invalidate_device(bdev, true))
> pr_warn("VFS: busy inodes on changed media %s\n",
> bdev->bd_disk->disk_name);
> - set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
> return true;
> }
> EXPORT_SYMBOL(bdev_check_media_change);
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 15eed210feeff4..2dca0aab0a9a25 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -315,7 +315,7 @@ static void nbd_size_update(struct nbd_device *nbd)
> bd_set_nr_sectors(bdev, nr_sectors);
> set_blocksize(bdev, config->blksize);
> } else
> - set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + set_bit(GD_NEED_PART_SCAN, &nbd->disk->state);
> bdput(bdev);
> }
> kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
> @@ -1322,7 +1322,7 @@ static int nbd_start_device_ioctl(struct nbd_device *nbd, struct block_device *b
> return ret;
>
> if (max_part)
> - set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + set_bit(GD_NEED_PART_SCAN, &nbd->disk->state);
> mutex_unlock(&nbd->config_lock);
> ret = wait_event_interruptible(config->recv_wq,
> atomic_read(&config->recv_threads) == 0);
> @@ -1500,9 +1500,9 @@ static int nbd_open(struct block_device *bdev, fmode_t mode)
> refcount_set(&nbd->config_refs, 1);
> refcount_inc(&nbd->refs);
> mutex_unlock(&nbd->config_lock);
> - set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
> } else if (nbd_disconnected(nbd->config)) {
> - set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
> }
> out:
> mutex_unlock(&nbd_index_mutex);
> diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
> index 661e2aa9c96784..e2b6c82586ce8b 100644
> --- a/drivers/ide/ide-gd.c
> +++ b/drivers/ide/ide-gd.c
> @@ -230,7 +230,7 @@ static int ide_gd_open(struct block_device *bdev, fmode_t mode)
> bdev->bd_disk->disk_name);
> drive->disk_ops->get_capacity(drive);
> set_capacity(disk, ide_gd_capacity(drive));
> - set_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + set_bit(GD_NEED_PART_SCAN, &disk->state);
> } else if (drive->dev_flags & IDE_DFLAG_FORMAT_IN_PROGRESS) {
> ret = -EBUSY;
> goto out_put_idkp;
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 0b34955b9e360f..1a9325f4315769 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -910,7 +910,6 @@ struct block_device *bdget(dev_t dev)
> bdev->bd_super = NULL;
> bdev->bd_inode = inode;
> bdev->bd_part_count = 0;
> - bdev->bd_flags = 0;
> inode->i_mode = S_IFBLK;
> inode->i_rdev = dev;
> inode->i_bdev = bdev;
> @@ -1385,7 +1384,7 @@ int bdev_disk_changed(struct block_device *bdev, bool invalidate)
>
> lockdep_assert_held(&bdev->bd_mutex);
>
> - clear_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags);
> + clear_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
>
> rescan:
> ret = blk_drop_partitions(bdev);
> @@ -1509,7 +1508,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, void *holder,
> * The latter is necessary to prevent ghost
> * partitions on a removed medium.
> */
> - if (test_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags) &&
> + if (test_bit(GD_NEED_PART_SCAN, &disk->state) &&
> (!ret || ret == -ENOMEDIUM))
> bdev_disk_changed(bdev, ret == -ENOMEDIUM);
>
> @@ -1539,7 +1538,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, void *holder,
> if (bdev->bd_disk->fops->open)
> ret = bdev->bd_disk->fops->open(bdev, mode);
> /* the same as first opener case, read comment there */
> - if (test_bit(BDEV_NEED_PART_SCAN, &bdev->bd_flags) &&
> + if (test_bit(GD_NEED_PART_SCAN, &disk->state) &&
> (!ret || ret == -ENOMEDIUM))
> bdev_disk_changed(bdev, ret == -ENOMEDIUM);
> if (ret)
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 6ffa783e16335e..eb20e28184ab19 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -19,8 +19,6 @@ struct cgroup_subsys_state;
> typedef void (bio_end_io_t) (struct bio *);
> struct bio_crypt_ctx;
>
> -#define BDEV_NEED_PART_SCAN 0
> -
> struct block_device {
> dev_t bd_dev;
> int bd_openers;
> @@ -39,7 +37,7 @@ struct block_device {
> struct hd_struct * bd_part;
> /* number of times partitions within this device have been opened. */
> unsigned bd_part_count;
> - unsigned long bd_flags;
> +
> spinlock_t bd_size_lock; /* for bd_inode->i_size updates */
> struct gendisk * bd_disk;
> struct backing_dev_info *bd_bdi;
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 1c97cf84f011a7..38f23d75701379 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -191,6 +191,8 @@ struct gendisk {
> void *private_data;
>
> int flags;
> + unsigned long state;
> +#define GD_NEED_PART_SCAN 0
> struct rw_semaphore lookup_sem;
> struct kobject *slave_dir;
>
> --
> 2.28.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR