From 1de797bb248d2276337139fecaffbd3bbc0f736d Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Thu, 12 Oct 2017 12:35:19 +0200 Subject: rbd: fix and simplify rbd_ioctl_set_ro() ->open_count/-EBUSY check is bogus and wrong: when an open device is set read-only, blkdev_write_iter() refuses further writes with -EPERM. This is standard behaviour and all other block devices allow this. set_disk_ro() call is also problematic: we affect the entire device when called on a single partition. All rbd_ioctl_set_ro() needs to do is refuse ro -> rw transition for mapped snapshots. Everything else can be handled by generic code. Signed-off-by: Ilya Dryomov --- drivers/block/rbd.c | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index adc877dfef5c..fb7cb38a6d83 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -640,46 +640,24 @@ static void rbd_release(struct gendisk *disk, fmode_t mode) static int rbd_ioctl_set_ro(struct rbd_device *rbd_dev, unsigned long arg) { - int ret = 0; - int val; - bool ro; - bool ro_changed = false; + int ro; - /* get_user() may sleep, so call it before taking rbd_dev->lock */ - if (get_user(val, (int __user *)(arg))) + if (get_user(ro, (int __user *)arg)) return -EFAULT; - ro = val ? true : false; - /* Snapshot doesn't allow to write*/ + /* Snapshots can't be marked read-write */ if (rbd_dev->spec->snap_id != CEPH_NOSNAP && !ro) return -EROFS; - spin_lock_irq(&rbd_dev->lock); - /* prevent others open this device */ - if (rbd_dev->open_count > 1) { - ret = -EBUSY; - goto out; - } - - if (rbd_dev->mapping.read_only != ro) { - rbd_dev->mapping.read_only = ro; - ro_changed = true; - } - -out: - spin_unlock_irq(&rbd_dev->lock); - /* set_disk_ro() may sleep, so call it after releasing rbd_dev->lock */ - if (ret == 0 && ro_changed) - set_disk_ro(rbd_dev->disk, ro ? 1 : 0); - - return ret; + /* Let blkdev_roset() handle it */ + return -ENOTTY; } static int rbd_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg) { struct rbd_device *rbd_dev = bdev->bd_disk->private_data; - int ret = 0; + int ret; switch (cmd) { case BLKROSET: -- cgit v1.2.3-59-g8ed1b From 9568c93ecab92d3ee60f2f6bec4e4d91641c61a6 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Thu, 12 Oct 2017 12:35:19 +0200 Subject: rbd: get rid of rbd_mapping::read_only It is redundant -- rw/ro state is stored in hd_struct and managed by the block layer. Signed-off-by: Ilya Dryomov --- drivers/block/rbd.c | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index fb7cb38a6d83..53b1ced21a13 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -348,7 +348,6 @@ struct rbd_client_id { struct rbd_mapping { u64 size; u64 features; - bool read_only; }; /* @@ -608,9 +607,6 @@ static int rbd_open(struct block_device *bdev, fmode_t mode) struct rbd_device *rbd_dev = bdev->bd_disk->private_data; bool removing = false; - if ((mode & FMODE_WRITE) && rbd_dev->mapping.read_only) - return -EROFS; - spin_lock_irq(&rbd_dev->lock); if (test_bit(RBD_DEV_FLAG_REMOVING, &rbd_dev->flags)) removing = true; @@ -4028,15 +4024,8 @@ static void rbd_queue_workfn(struct work_struct *work) goto err_rq; } - /* Only reads are allowed to a read-only device */ - - if (op_type != OBJ_OP_READ) { - if (rbd_dev->mapping.read_only) { - result = -EROFS; - goto err_rq; - } - rbd_assert(rbd_dev->spec->snap_id == CEPH_NOSNAP); - } + rbd_assert(op_type == OBJ_OP_READ || + rbd_dev->spec->snap_id == CEPH_NOSNAP); /* * Quit early if the mapped snapshot no longer exists. It's @@ -5972,7 +5961,7 @@ static int rbd_dev_device_setup(struct rbd_device *rbd_dev) goto err_out_disk; set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE); - set_disk_ro(rbd_dev->disk, rbd_dev->mapping.read_only); + set_disk_ro(rbd_dev->disk, rbd_dev->opts->read_only); ret = dev_set_name(&rbd_dev->dev, "%d", rbd_dev->dev_id); if (ret) @@ -6123,7 +6112,6 @@ static ssize_t do_rbd_add(struct bus_type *bus, struct rbd_options *rbd_opts = NULL; struct rbd_spec *spec = NULL; struct rbd_client *rbdc; - bool read_only; int rc; if (!try_module_get(THIS_MODULE)) @@ -6172,11 +6160,8 @@ static ssize_t do_rbd_add(struct bus_type *bus, } /* If we are mapping a snapshot it must be marked read-only */ - - read_only = rbd_dev->opts->read_only; if (rbd_dev->spec->snap_id != CEPH_NOSNAP) - read_only = true; - rbd_dev->mapping.read_only = read_only; + rbd_dev->opts->read_only = true; rc = rbd_dev_device_setup(rbd_dev); if (rc) -- cgit v1.2.3-59-g8ed1b From 7c084289795bc0f3b9ab315ac3c8d269dd4d0215 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 2 Nov 2017 01:05:11 +0100 Subject: rbd: set discard_alignment to zero RBD devices are currently incorrectly initialised with the block queue discard_alignment set to the underlying RADOS object size. As per Documentation/ABI/testing/sysfs-block: The discard_alignment parameter indicates how many bytes the beginning of the device is offset from the internal allocation unit's natural alignment. Correcting the discard_alignment parameter from the RADOS object size to zero (the blk_set_default_limits() default) has no effect on how discard requests are propagated through the block layer - @alignment in __blkdev_issue_discard() remains zero. However, it does fix the UNMAP granularity alignment value advertised to SCSI initiators via the Block Limits VPD. Signed-off-by: David Disseldorp Reviewed-by: Ilya Dryomov Signed-off-by: Ilya Dryomov --- drivers/block/rbd.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/block') diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 53b1ced21a13..8c132a7fbd2c 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -4390,7 +4390,6 @@ static int rbd_init_disk(struct rbd_device *rbd_dev) /* enable the discard support */ queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q); q->limits.discard_granularity = segment_size; - q->limits.discard_alignment = segment_size; blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE); blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE); -- cgit v1.2.3-59-g8ed1b From 3cfa3b16dd2f1787f9d19d6da2fe9652d806b387 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 13 Nov 2017 10:35:40 +0100 Subject: rbd: default to single-major device number scheme It's been 3.5 years, let's turn it on by default. Support in rbd(8) utility goes back to pre-firefly, "rbd map" has been loading the module with single_major=Y ever since. However, if the module is already loaded (whether by hand or at boot time), we end up with single_major=N. Also, some people don't install rbd(8) and use the sysfs interface directly. (With single-major=N, a major number is consumed for every mapping, imposing a limit of ~240 rbd images per host. single-major=Y allows mapping thousands of rbd images on a single machine.) Signed-off-by: Ilya Dryomov Reviewed-by: Jason Dillaman --- drivers/block/rbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 8c132a7fbd2c..38fc5f397fde 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -449,12 +449,11 @@ static DEFINE_IDA(rbd_dev_id_ida); static struct workqueue_struct *rbd_wq; /* - * Default to false for now, as single-major requires >= 0.75 version of - * userspace rbd utility. + * single-major requires >= 0.75 version of userspace rbd utility. */ -static bool single_major = false; +static bool single_major = true; module_param(single_major, bool, S_IRUGO); -MODULE_PARM_DESC(single_major, "Use a single major number for all rbd devices (default: false)"); +MODULE_PARM_DESC(single_major, "Use a single major number for all rbd devices (default: true)"); static int rbd_img_request_submit(struct rbd_img_request *img_request); -- cgit v1.2.3-59-g8ed1b