From: Jeff Moyer <jmoyer@redhat.com> To: Ming Lei <ming.lei@redhat.com> Cc: Jens Axboe <axboe@kernel.dk>, Mike Snitzer <snitzer@redhat.com>, linux-block@vger.kernel.org, dm-devel@redhat.com Subject: Re: [dm-devel] [PATCH 3/3] dm: mark dm queue as blocking if any underlying is blocking Date: Tue, 11 Jan 2022 13:14:41 -0500 [thread overview] Message-ID: <x49iluqjg1a.fsf@segfault.boston.devel.redhat.com> (raw) In-Reply-To: <20211221141459.1368176-4-ming.lei@redhat.com> (Ming Lei's message of "Tue, 21 Dec 2021 22:14:59 +0800") Ming Lei <ming.lei@redhat.com> writes: > dm request based driver doesn't set BLK_MQ_F_BLOCKING, so dm_queue_rq() > is supposed to not sleep. > > However, blk_insert_cloned_request() is used by dm_queue_rq() for > queuing underlying request, but the underlying queue may be marked as > BLK_MQ_F_BLOCKING, so blk_insert_cloned_request() may become to block > current context, then rcu warning is triggered. > > Fixes the issue by marking dm request based queue as BLK_MQ_F_BLOCKING > if any underlying queue is marked as BLK_MQ_F_BLOCKING, meantime we > need to allocate srcu beforehand. > > Signed-off-by: Ming Lei <ming.lei@redhat.com> Acked-by: Jeff Moyer <jmoyer@redhat.com> > --- > drivers/md/dm-rq.c | 5 ++++- > drivers/md/dm-rq.h | 3 ++- > drivers/md/dm-table.c | 14 ++++++++++++++ > drivers/md/dm.c | 5 +++-- > drivers/md/dm.h | 1 + > 5 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c > index 579ab6183d4d..2297d37c62a9 100644 > --- a/drivers/md/dm-rq.c > +++ b/drivers/md/dm-rq.c > @@ -535,7 +535,8 @@ static const struct blk_mq_ops dm_mq_ops = { > .init_request = dm_mq_init_request, > }; > > -int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t) > +int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t, > + bool blocking) > { > struct dm_target *immutable_tgt; > int err; > @@ -550,6 +551,8 @@ int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t) > md->tag_set->flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_STACKING; > md->tag_set->nr_hw_queues = dm_get_blk_mq_nr_hw_queues(); > md->tag_set->driver_data = md; > + if (blocking) > + md->tag_set->flags |= BLK_MQ_F_BLOCKING; > > md->tag_set->cmd_size = sizeof(struct dm_rq_target_io); > immutable_tgt = dm_table_get_immutable_target(t); > diff --git a/drivers/md/dm-rq.h b/drivers/md/dm-rq.h > index 1eea0da641db..5f3729f277d7 100644 > --- a/drivers/md/dm-rq.h > +++ b/drivers/md/dm-rq.h > @@ -30,7 +30,8 @@ struct dm_rq_clone_bio_info { > struct bio clone; > }; > > -int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t); > +int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t, > + bool blocking); > void dm_mq_cleanup_mapped_device(struct mapped_device *md); > > void dm_start_queue(struct request_queue *q); > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c > index aa173f5bdc3d..e4bdd4f757a3 100644 > --- a/drivers/md/dm-table.c > +++ b/drivers/md/dm-table.c > @@ -1875,6 +1875,20 @@ static bool dm_table_supports_write_zeroes(struct dm_table *t) > return true; > } > > +/* If the device can block inside ->queue_rq */ > +static int device_is_io_blocking(struct dm_target *ti, struct dm_dev *dev, > + sector_t start, sector_t len, void *data) > +{ > + struct request_queue *q = bdev_get_queue(dev->bdev); > + > + return blk_queue_blocking(q); > +} > + > +bool dm_table_has_blocking_dev(struct dm_table *t) > +{ > + return dm_table_any_dev_attr(t, device_is_io_blocking, NULL); > +} > + > static int device_not_nowait_capable(struct dm_target *ti, struct dm_dev *dev, > sector_t start, sector_t len, void *data) > { > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > index 280918cdcabd..2f72877752dd 100644 > --- a/drivers/md/dm.c > +++ b/drivers/md/dm.c > @@ -1761,7 +1761,7 @@ static struct mapped_device *alloc_dev(int minor) > * established. If request-based table is loaded: blk-mq will > * override accordingly. > */ > - md->disk = blk_alloc_disk(md->numa_node_id); > + md->disk = blk_alloc_disk_srcu(md->numa_node_id); > if (!md->disk) > goto bad; > md->queue = md->disk->queue; > @@ -2046,7 +2046,8 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t) > switch (type) { > case DM_TYPE_REQUEST_BASED: > md->disk->fops = &dm_rq_blk_dops; > - r = dm_mq_init_request_queue(md, t); > + r = dm_mq_init_request_queue(md, t, > + dm_table_has_blocking_dev(t)); > if (r) { > DMERR("Cannot initialize queue for request-based dm mapped device"); > return r; > diff --git a/drivers/md/dm.h b/drivers/md/dm.h > index 742d9c80efe1..f7f92b272cce 100644 > --- a/drivers/md/dm.h > +++ b/drivers/md/dm.h > @@ -60,6 +60,7 @@ int dm_calculate_queue_limits(struct dm_table *table, > struct queue_limits *limits); > int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, > struct queue_limits *limits); > +bool dm_table_has_blocking_dev(struct dm_table *t); > struct list_head *dm_table_get_devices(struct dm_table *t); > void dm_table_presuspend_targets(struct dm_table *t); > void dm_table_presuspend_undo_targets(struct dm_table *t);
WARNING: multiple messages have this Message-ID (diff)
From: Jeff Moyer <jmoyer@redhat.com> To: Ming Lei <ming.lei@redhat.com> Cc: Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org, dm-devel@redhat.com, Mike Snitzer <snitzer@redhat.com> Subject: Re: [dm-devel] [PATCH 3/3] dm: mark dm queue as blocking if any underlying is blocking Date: Tue, 11 Jan 2022 13:14:41 -0500 [thread overview] Message-ID: <x49iluqjg1a.fsf@segfault.boston.devel.redhat.com> (raw) In-Reply-To: <20211221141459.1368176-4-ming.lei@redhat.com> (Ming Lei's message of "Tue, 21 Dec 2021 22:14:59 +0800") Ming Lei <ming.lei@redhat.com> writes: > dm request based driver doesn't set BLK_MQ_F_BLOCKING, so dm_queue_rq() > is supposed to not sleep. > > However, blk_insert_cloned_request() is used by dm_queue_rq() for > queuing underlying request, but the underlying queue may be marked as > BLK_MQ_F_BLOCKING, so blk_insert_cloned_request() may become to block > current context, then rcu warning is triggered. > > Fixes the issue by marking dm request based queue as BLK_MQ_F_BLOCKING > if any underlying queue is marked as BLK_MQ_F_BLOCKING, meantime we > need to allocate srcu beforehand. > > Signed-off-by: Ming Lei <ming.lei@redhat.com> Acked-by: Jeff Moyer <jmoyer@redhat.com> > --- > drivers/md/dm-rq.c | 5 ++++- > drivers/md/dm-rq.h | 3 ++- > drivers/md/dm-table.c | 14 ++++++++++++++ > drivers/md/dm.c | 5 +++-- > drivers/md/dm.h | 1 + > 5 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c > index 579ab6183d4d..2297d37c62a9 100644 > --- a/drivers/md/dm-rq.c > +++ b/drivers/md/dm-rq.c > @@ -535,7 +535,8 @@ static const struct blk_mq_ops dm_mq_ops = { > .init_request = dm_mq_init_request, > }; > > -int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t) > +int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t, > + bool blocking) > { > struct dm_target *immutable_tgt; > int err; > @@ -550,6 +551,8 @@ int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t) > md->tag_set->flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_STACKING; > md->tag_set->nr_hw_queues = dm_get_blk_mq_nr_hw_queues(); > md->tag_set->driver_data = md; > + if (blocking) > + md->tag_set->flags |= BLK_MQ_F_BLOCKING; > > md->tag_set->cmd_size = sizeof(struct dm_rq_target_io); > immutable_tgt = dm_table_get_immutable_target(t); > diff --git a/drivers/md/dm-rq.h b/drivers/md/dm-rq.h > index 1eea0da641db..5f3729f277d7 100644 > --- a/drivers/md/dm-rq.h > +++ b/drivers/md/dm-rq.h > @@ -30,7 +30,8 @@ struct dm_rq_clone_bio_info { > struct bio clone; > }; > > -int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t); > +int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t, > + bool blocking); > void dm_mq_cleanup_mapped_device(struct mapped_device *md); > > void dm_start_queue(struct request_queue *q); > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c > index aa173f5bdc3d..e4bdd4f757a3 100644 > --- a/drivers/md/dm-table.c > +++ b/drivers/md/dm-table.c > @@ -1875,6 +1875,20 @@ static bool dm_table_supports_write_zeroes(struct dm_table *t) > return true; > } > > +/* If the device can block inside ->queue_rq */ > +static int device_is_io_blocking(struct dm_target *ti, struct dm_dev *dev, > + sector_t start, sector_t len, void *data) > +{ > + struct request_queue *q = bdev_get_queue(dev->bdev); > + > + return blk_queue_blocking(q); > +} > + > +bool dm_table_has_blocking_dev(struct dm_table *t) > +{ > + return dm_table_any_dev_attr(t, device_is_io_blocking, NULL); > +} > + > static int device_not_nowait_capable(struct dm_target *ti, struct dm_dev *dev, > sector_t start, sector_t len, void *data) > { > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > index 280918cdcabd..2f72877752dd 100644 > --- a/drivers/md/dm.c > +++ b/drivers/md/dm.c > @@ -1761,7 +1761,7 @@ static struct mapped_device *alloc_dev(int minor) > * established. If request-based table is loaded: blk-mq will > * override accordingly. > */ > - md->disk = blk_alloc_disk(md->numa_node_id); > + md->disk = blk_alloc_disk_srcu(md->numa_node_id); > if (!md->disk) > goto bad; > md->queue = md->disk->queue; > @@ -2046,7 +2046,8 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t) > switch (type) { > case DM_TYPE_REQUEST_BASED: > md->disk->fops = &dm_rq_blk_dops; > - r = dm_mq_init_request_queue(md, t); > + r = dm_mq_init_request_queue(md, t, > + dm_table_has_blocking_dev(t)); > if (r) { > DMERR("Cannot initialize queue for request-based dm mapped device"); > return r; > diff --git a/drivers/md/dm.h b/drivers/md/dm.h > index 742d9c80efe1..f7f92b272cce 100644 > --- a/drivers/md/dm.h > +++ b/drivers/md/dm.h > @@ -60,6 +60,7 @@ int dm_calculate_queue_limits(struct dm_table *table, > struct queue_limits *limits); > int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, > struct queue_limits *limits); > +bool dm_table_has_blocking_dev(struct dm_table *t); > struct list_head *dm_table_get_devices(struct dm_table *t); > void dm_table_presuspend_targets(struct dm_table *t); > void dm_table_presuspend_undo_targets(struct dm_table *t); -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2022-01-11 18:12 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-21 14:14 [PATCH 0/3] blk-mq/dm-rq: support BLK_MQ_F_BLOCKING for dm-rq Ming Lei 2021-12-21 14:14 ` [dm-devel] " Ming Lei 2021-12-21 14:14 ` [PATCH 1/3] block: split having srcu from queue blocking Ming Lei 2021-12-21 14:14 ` [dm-devel] " Ming Lei 2022-01-11 18:13 ` Jeff Moyer 2022-01-11 18:13 ` Jeff Moyer 2021-12-21 14:14 ` [PATCH 2/3] block: add blk_alloc_disk_srcu Ming Lei 2021-12-21 14:14 ` [dm-devel] " Ming Lei 2022-01-11 18:13 ` Jeff Moyer 2022-01-11 18:13 ` Jeff Moyer 2021-12-21 14:14 ` [PATCH 3/3] dm: mark dm queue as blocking if any underlying is blocking Ming Lei 2021-12-21 14:14 ` [dm-devel] " Ming Lei 2022-01-06 15:40 ` Mike Snitzer 2022-01-06 15:40 ` [dm-devel] " Mike Snitzer 2022-01-06 15:51 ` Ming Lei 2022-01-06 15:51 ` [dm-devel] " Ming Lei 2022-01-10 19:23 ` Mike Snitzer 2022-01-10 19:23 ` [dm-devel] " Mike Snitzer 2022-01-11 18:14 ` Jeff Moyer [this message] 2022-01-11 18:14 ` Jeff Moyer 2021-12-21 16:21 ` [PATCH 0/3] blk-mq/dm-rq: support BLK_MQ_F_BLOCKING for dm-rq Christoph Hellwig 2021-12-21 16:21 ` [dm-devel] " Christoph Hellwig 2021-12-23 4:16 ` Ming Lei 2021-12-23 4:16 ` [dm-devel] " Ming Lei 2021-12-28 21:30 ` Mike Snitzer 2021-12-28 21:30 ` [dm-devel] " Mike Snitzer 2022-01-10 19:26 ` Mike Snitzer 2022-01-10 19:26 ` [dm-devel] " Mike Snitzer 2022-01-11 8:34 ` Christoph Hellwig 2022-01-11 8:34 ` Christoph Hellwig 2022-01-11 16:15 ` Mike Snitzer 2022-01-11 16:15 ` [dm-devel] " Mike Snitzer 2022-01-17 8:08 ` Christoph Hellwig 2022-01-17 8:08 ` Christoph Hellwig 2022-01-11 18:23 ` Jeff Moyer 2022-01-11 18:23 ` Jeff Moyer 2022-01-17 8:10 ` Christoph Hellwig 2022-01-17 8:10 ` Christoph Hellwig 2022-01-19 21:03 ` Mike Snitzer 2022-01-19 21:03 ` [dm-devel] " Mike Snitzer
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=x49iluqjg1a.fsf@segfault.boston.devel.redhat.com \ --to=jmoyer@redhat.com \ --cc=axboe@kernel.dk \ --cc=dm-devel@redhat.com \ --cc=linux-block@vger.kernel.org \ --cc=ming.lei@redhat.com \ --cc=snitzer@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.