On Wed, Jan 10, 2018 at 5:34 PM, Jens Axboe [off-list ref] wrote:

On 1/10/18 9:18 AM, Ilya Dryomov wrote:

quoted

Regular block device writes go through blkdev_write_iter(), which does
bdev_read_only(), while zeroout/discard/etc requests are never checked,
both userspace- and kernel-triggered.  Add a generic catch-all check to
generic_make_request_checks() to actually enforce ioctl(BLKROSET) and
set_disk_ro(), which is used by quite a few drivers for things like
snapshots, read-only backing files/images, etc.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 block/blk-core.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index f843ae4f858d..d132bec4a266 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c

@@ -2123,6 +2123,20 @@ static inline int bio_check_eod(struct bio *bio, unsigned int nr_sectors)
      return 0;
 }

+static inline bool bio_check_ro(struct bio *bio)
+{
+     struct hd_struct *p;
+     bool ret = false;
+
+     rcu_read_lock();
+     p = __disk_get_part(bio->bi_disk, bio->bi_partno);
+     if (!p || (p->policy && op_is_write(bio_op(bio))))
+             ret = true;
+     rcu_read_unlock();
+
+     return ret;
+}> +
 static noinline_for_stack bool
 generic_make_request_checks(struct bio *bio)
 {

@@ -2145,11 +2159,18 @@ generic_make_request_checks(struct bio *bio)
              goto end_io;
      }

+     if (unlikely(bio_check_ro(bio))) {
+             printk(KERN_ERR
+                    "generic_make_request: Trying to write "
+                     "to read-only block-device %s (partno %d)\n",
+                     bio_devname(bio, b), bio->bi_partno);
+             goto end_io;
+     }

It's yet another check that adds part lookup and rcu lock/unlock in that
path. Can we combine some of them? Make this part of the remap?  This
overhead impacts every IO, let's not bloat it more than absolutely
necessary.

Yes, combining with should_fail_request check in remap should be easy
enough.  I considered it, but opted for the less invasive patch.  I'll
re-spin.

Thanks,

                Ilya