* [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED [not found] <20190215020855.176727-1-sashal@kernel.org> @ 2019-02-15 2:08 ` Sasha Levin 2019-02-15 2:24 ` Tetsuo Handa 2019-02-15 2:28 ` Ming Lei 2019-02-15 2:08 ` [PATCH AUTOSEL 4.20 77/77] Revert "block: cover another queue enter recursion via BIO_QUEUE_ENTERED" Sasha Levin 1 sibling, 2 replies; 5+ messages in thread From: Sasha Levin @ 2019-02-15 2:08 UTC (permalink / raw) To: linux-kernel, stable Cc: Ming Lei, Tetsuo Handa, NeilBrown, Jens Axboe, Sasha Levin, linux-block From: Ming Lei <ming.lei@redhat.com> [ Upstream commit 698cef173983b086977e633e46476e0f925ca01e ] Except for blk_queue_split(), bio_split() is used for splitting bio too, then the remained bio is often resubmit to queue via generic_make_request(). So the same queue enter recursion exits in this case too. Unfortunatley commit cd4a4ae4683dc2 doesn't help this case. This patch covers the above case by setting BIO_QUEUE_ENTERED before calling q->make_request_fn. In theory the per-bio flag is used to simulate one stack variable, it is just fine to clear it after q->make_request_fn is returned. Especially the same bio can't be submitted from another context. Fixes: cd4a4ae4683dc2 ("block: don't use blocking queue entered for recursive bio submits") Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: NeilBrown <neilb@suse.com> Reviewed-by: Mike Snitzer <snitzer@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Sasha Levin <sashal@kernel.org> --- block/blk-core.c | 11 +++++++++++ block/blk-merge.c | 10 ---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index deb56932f8c4..22260339f66f 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -2449,7 +2449,18 @@ blk_qc_t generic_make_request(struct bio *bio) /* Create a fresh bio_list for all subordinate requests */ bio_list_on_stack[1] = bio_list_on_stack[0]; bio_list_init(&bio_list_on_stack[0]); + + /* + * Since we're recursing into make_request here, ensure + * that we mark this bio as already having entered the queue. + * If not, and the queue is going away, we can get stuck + * forever on waiting for the queue reference to drop. But + * that will never happen, as we're already holding a + * reference to it. + */ + bio_set_flag(bio, BIO_QUEUE_ENTERED); ret = q->make_request_fn(q, bio); + bio_clear_flag(bio, BIO_QUEUE_ENTERED); /* sort new bios into those for a lower level * and those for the same level diff --git a/block/blk-merge.c b/block/blk-merge.c index 7695034f4b87..adfe835ab258 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -272,16 +272,6 @@ void blk_queue_split(struct request_queue *q, struct bio **bio) /* there isn't chance to merge the splitted bio */ split->bi_opf |= REQ_NOMERGE; - /* - * Since we're recursing into make_request here, ensure - * that we mark this bio as already having entered the queue. - * If not, and the queue is going away, we can get stuck - * forever on waiting for the queue reference to drop. But - * that will never happen, as we're already holding a - * reference to it. - */ - bio_set_flag(*bio, BIO_QUEUE_ENTERED); - bio_chain(split, *bio); trace_block_split(q, split, (*bio)->bi_iter.bi_sector); generic_make_request(*bio); -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED 2019-02-15 2:08 ` [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED Sasha Levin @ 2019-02-15 2:24 ` Tetsuo Handa 2019-02-15 2:28 ` Ming Lei 1 sibling, 0 replies; 5+ messages in thread From: Tetsuo Handa @ 2019-02-15 2:24 UTC (permalink / raw) To: Sasha Levin Cc: linux-kernel, stable, Ming Lei, Tetsuo Handa, NeilBrown, Jens Axboe, Sasha Levin, linux-block This patch turned out to be wrong and was reverted. Please drop this patch. commit 947b7ac135b16aa60f9141ff72bd494eda0edb5e Author: Jens Axboe <axboe@kernel.dk> Date: Sun Jan 27 06:35:28 2019 -0700 Revert "block: cover another queue enter recursion via BIO_QUEUE_ENTERED" We can't touch a bio after ->make_request_fn(), for all we know it could already have been completed by the time this function returns. This reverts commit 698cef173983b086977e633e46476e0f925ca01e. Reported-by: syzbot+4df6ca820108fd248943@syzkaller.appspotmail.com Signed-off-by: Jens Axboe <axboe@kernel.dk> > From: Ming Lei <ming.lei@redhat.com> > > [ Upstream commit 698cef173983b086977e633e46476e0f925ca01e ] > > Except for blk_queue_split(), bio_split() is used for splitting bio too, > then the remained bio is often resubmit to queue via generic_make_request(). > So the same queue enter recursion exits in this case too. Unfortunatley > commit cd4a4ae4683dc2 doesn't help this case. > > This patch covers the above case by setting BIO_QUEUE_ENTERED before calling > q->make_request_fn. > > In theory the per-bio flag is used to simulate one stack variable, it is > just fine to clear it after q->make_request_fn is returned. Especially > the same bio can't be submitted from another context. > > Fixes: cd4a4ae4683dc2 ("block: don't use blocking queue entered for recursive bio submits") > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: NeilBrown <neilb@suse.com> > Reviewed-by: Mike Snitzer <snitzer@redhat.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Jens Axboe <axboe@kernel.dk> > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > block/blk-core.c | 11 +++++++++++ > block/blk-merge.c | 10 ---------- > 2 files changed, 11 insertions(+), 10 deletions(-) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED 2019-02-15 2:08 ` [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED Sasha Levin 2019-02-15 2:24 ` Tetsuo Handa @ 2019-02-15 2:28 ` Ming Lei 2019-02-27 17:39 ` Sasha Levin 1 sibling, 1 reply; 5+ messages in thread From: Ming Lei @ 2019-02-15 2:28 UTC (permalink / raw) To: Sasha Levin Cc: linux-kernel, stable, Tetsuo Handa, NeilBrown, Jens Axboe, linux-block On Thu, Feb 14, 2019 at 09:08:27PM -0500, Sasha Levin wrote: > From: Ming Lei <ming.lei@redhat.com> > > [ Upstream commit 698cef173983b086977e633e46476e0f925ca01e ] > > Except for blk_queue_split(), bio_split() is used for splitting bio too, > then the remained bio is often resubmit to queue via generic_make_request(). > So the same queue enter recursion exits in this case too. Unfortunatley > commit cd4a4ae4683dc2 doesn't help this case. > > This patch covers the above case by setting BIO_QUEUE_ENTERED before calling > q->make_request_fn. > > In theory the per-bio flag is used to simulate one stack variable, it is > just fine to clear it after q->make_request_fn is returned. Especially > the same bio can't be submitted from another context. > > Fixes: cd4a4ae4683dc2 ("block: don't use blocking queue entered for recursive bio submits") > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: NeilBrown <neilb@suse.com> > Reviewed-by: Mike Snitzer <snitzer@redhat.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Jens Axboe <axboe@kernel.dk> > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > block/blk-core.c | 11 +++++++++++ > block/blk-merge.c | 10 ---------- > 2 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index deb56932f8c4..22260339f66f 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -2449,7 +2449,18 @@ blk_qc_t generic_make_request(struct bio *bio) > /* Create a fresh bio_list for all subordinate requests */ > bio_list_on_stack[1] = bio_list_on_stack[0]; > bio_list_init(&bio_list_on_stack[0]); > + > + /* > + * Since we're recursing into make_request here, ensure > + * that we mark this bio as already having entered the queue. > + * If not, and the queue is going away, we can get stuck > + * forever on waiting for the queue reference to drop. But > + * that will never happen, as we're already holding a > + * reference to it. > + */ > + bio_set_flag(bio, BIO_QUEUE_ENTERED); > ret = q->make_request_fn(q, bio); > + bio_clear_flag(bio, BIO_QUEUE_ENTERED); > > /* sort new bios into those for a lower level > * and those for the same level > diff --git a/block/blk-merge.c b/block/blk-merge.c > index 7695034f4b87..adfe835ab258 100644 > --- a/block/blk-merge.c > +++ b/block/blk-merge.c > @@ -272,16 +272,6 @@ void blk_queue_split(struct request_queue *q, struct bio **bio) > /* there isn't chance to merge the splitted bio */ > split->bi_opf |= REQ_NOMERGE; > > - /* > - * Since we're recursing into make_request here, ensure > - * that we mark this bio as already having entered the queue. > - * If not, and the queue is going away, we can get stuck > - * forever on waiting for the queue reference to drop. But > - * that will never happen, as we're already holding a > - * reference to it. > - */ > - bio_set_flag(*bio, BIO_QUEUE_ENTERED); > - > bio_chain(split, *bio); > trace_block_split(q, split, (*bio)->bi_iter.bi_sector); > generic_make_request(*bio); > -- > 2.19.1 > This one should be dropped, given it has been reverted. -- Ming ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED 2019-02-15 2:28 ` Ming Lei @ 2019-02-27 17:39 ` Sasha Levin 0 siblings, 0 replies; 5+ messages in thread From: Sasha Levin @ 2019-02-27 17:39 UTC (permalink / raw) To: Ming Lei Cc: linux-kernel, stable, Tetsuo Handa, NeilBrown, Jens Axboe, linux-block On Fri, Feb 15, 2019 at 10:28:08AM +0800, Ming Lei wrote: >On Thu, Feb 14, 2019 at 09:08:27PM -0500, Sasha Levin wrote: >> From: Ming Lei <ming.lei@redhat.com> >> >> [ Upstream commit 698cef173983b086977e633e46476e0f925ca01e ] >> >> Except for blk_queue_split(), bio_split() is used for splitting bio too, >> then the remained bio is often resubmit to queue via generic_make_request(). >> So the same queue enter recursion exits in this case too. Unfortunatley >> commit cd4a4ae4683dc2 doesn't help this case. >> >> This patch covers the above case by setting BIO_QUEUE_ENTERED before calling >> q->make_request_fn. >> >> In theory the per-bio flag is used to simulate one stack variable, it is >> just fine to clear it after q->make_request_fn is returned. Especially >> the same bio can't be submitted from another context. >> >> Fixes: cd4a4ae4683dc2 ("block: don't use blocking queue entered for recursive bio submits") >> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> Cc: NeilBrown <neilb@suse.com> >> Reviewed-by: Mike Snitzer <snitzer@redhat.com> >> Signed-off-by: Ming Lei <ming.lei@redhat.com> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> Signed-off-by: Sasha Levin <sashal@kernel.org> >> --- >> block/blk-core.c | 11 +++++++++++ >> block/blk-merge.c | 10 ---------- >> 2 files changed, 11 insertions(+), 10 deletions(-) >> >> diff --git a/block/blk-core.c b/block/blk-core.c >> index deb56932f8c4..22260339f66f 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -2449,7 +2449,18 @@ blk_qc_t generic_make_request(struct bio *bio) >> /* Create a fresh bio_list for all subordinate requests */ >> bio_list_on_stack[1] = bio_list_on_stack[0]; >> bio_list_init(&bio_list_on_stack[0]); >> + >> + /* >> + * Since we're recursing into make_request here, ensure >> + * that we mark this bio as already having entered the queue. >> + * If not, and the queue is going away, we can get stuck >> + * forever on waiting for the queue reference to drop. But >> + * that will never happen, as we're already holding a >> + * reference to it. >> + */ >> + bio_set_flag(bio, BIO_QUEUE_ENTERED); >> ret = q->make_request_fn(q, bio); >> + bio_clear_flag(bio, BIO_QUEUE_ENTERED); >> >> /* sort new bios into those for a lower level >> * and those for the same level >> diff --git a/block/blk-merge.c b/block/blk-merge.c >> index 7695034f4b87..adfe835ab258 100644 >> --- a/block/blk-merge.c >> +++ b/block/blk-merge.c >> @@ -272,16 +272,6 @@ void blk_queue_split(struct request_queue *q, struct bio **bio) >> /* there isn't chance to merge the splitted bio */ >> split->bi_opf |= REQ_NOMERGE; >> >> - /* >> - * Since we're recursing into make_request here, ensure >> - * that we mark this bio as already having entered the queue. >> - * If not, and the queue is going away, we can get stuck >> - * forever on waiting for the queue reference to drop. But >> - * that will never happen, as we're already holding a >> - * reference to it. >> - */ >> - bio_set_flag(*bio, BIO_QUEUE_ENTERED); >> - >> bio_chain(split, *bio); >> trace_block_split(q, split, (*bio)->bi_iter.bi_sector); >> generic_make_request(*bio); >> -- >> 2.19.1 >> > >This one should be dropped, given it has been reverted. Dropped, thank you. -- Thanks, Sasha ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH AUTOSEL 4.20 77/77] Revert "block: cover another queue enter recursion via BIO_QUEUE_ENTERED" [not found] <20190215020855.176727-1-sashal@kernel.org> 2019-02-15 2:08 ` [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED Sasha Levin @ 2019-02-15 2:08 ` Sasha Levin 1 sibling, 0 replies; 5+ messages in thread From: Sasha Levin @ 2019-02-15 2:08 UTC (permalink / raw) To: linux-kernel, stable; +Cc: Jens Axboe, Sasha Levin, linux-block From: Jens Axboe <axboe@kernel.dk> [ Upstream commit 947b7ac135b16aa60f9141ff72bd494eda0edb5e ] We can't touch a bio after ->make_request_fn(), for all we know it could already have been completed by the time this function returns. This reverts commit 698cef173983b086977e633e46476e0f925ca01e. Reported-by: syzbot+4df6ca820108fd248943@syzkaller.appspotmail.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Sasha Levin <sashal@kernel.org> --- block/blk-core.c | 11 ----------- block/blk-merge.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 22260339f66f..deb56932f8c4 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -2449,18 +2449,7 @@ blk_qc_t generic_make_request(struct bio *bio) /* Create a fresh bio_list for all subordinate requests */ bio_list_on_stack[1] = bio_list_on_stack[0]; bio_list_init(&bio_list_on_stack[0]); - - /* - * Since we're recursing into make_request here, ensure - * that we mark this bio as already having entered the queue. - * If not, and the queue is going away, we can get stuck - * forever on waiting for the queue reference to drop. But - * that will never happen, as we're already holding a - * reference to it. - */ - bio_set_flag(bio, BIO_QUEUE_ENTERED); ret = q->make_request_fn(q, bio); - bio_clear_flag(bio, BIO_QUEUE_ENTERED); /* sort new bios into those for a lower level * and those for the same level diff --git a/block/blk-merge.c b/block/blk-merge.c index adfe835ab258..7695034f4b87 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -272,6 +272,16 @@ void blk_queue_split(struct request_queue *q, struct bio **bio) /* there isn't chance to merge the splitted bio */ split->bi_opf |= REQ_NOMERGE; + /* + * Since we're recursing into make_request here, ensure + * that we mark this bio as already having entered the queue. + * If not, and the queue is going away, we can get stuck + * forever on waiting for the queue reference to drop. But + * that will never happen, as we're already holding a + * reference to it. + */ + bio_set_flag(*bio, BIO_QUEUE_ENTERED); + bio_chain(split, *bio); trace_block_split(q, split, (*bio)->bi_iter.bi_sector); generic_make_request(*bio); -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-27 17:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190215020855.176727-1-sashal@kernel.org>
2019-02-15 2:08 ` [PATCH AUTOSEL 4.20 49/77] block: cover another queue enter recursion via BIO_QUEUE_ENTERED Sasha Levin
2019-02-15 2:24 ` Tetsuo Handa
2019-02-15 2:28 ` Ming Lei
2019-02-27 17:39 ` Sasha Levin
2019-02-15 2:08 ` [PATCH AUTOSEL 4.20 77/77] Revert "block: cover another queue enter recursion via BIO_QUEUE_ENTERED" Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox