From: NeilBrown <neilb@suse.com> To: Michael Wang <yun.wang@profitbricks.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-block@vger.kernel.org, linux-raid@vger.kernel.org Cc: Jens Axboe <axboe@kernel.dk>, Shaohua Li <shli@kernel.org>, Jinpu Wang <jinpu.wang@profitbricks.com> Subject: Re: [RFC PATCH] blk: reset 'bi_next' when bio is done inside request Date: Tue, 04 Apr 2017 07:25:45 +1000 [thread overview] Message-ID: <877f31kwti.fsf@notabene.neil.brown.name> (raw) In-Reply-To: <9505ff12-7307-7dec-76b5-2a233a592634@profitbricks.com> [-- Attachment #1: Type: text/plain, Size: 1927 bytes --] On Mon, Apr 03 2017, Michael Wang wrote: > blk_attempt_plug_merge() try to merge bio into request and chain them > by 'bi_next', while after the bio is done inside request, we forgot to > reset the 'bi_next'. > > This lead into BUG while removing all the underlying devices from md-raid1, > the bio once go through: > > md_do_sync() > sync_request() > generic_make_request() This is a read request from the "first" device. > blk_queue_bio() > blk_attempt_plug_merge() > CHAINED HERE > > will keep chained and reused by: > > raid1d() > sync_request_write() > generic_make_request() This is a write request to some other device, isn't it? If sync_request_write() is using a bio that has already been used, it should call bio_reset() and fill in the details again. However I don't see how that would happen. Can you give specific details on the situation that triggers the bug? Thanks, NeilBrown > BUG_ON(bio->bi_next) > > After reset the 'bi_next' this can no longer happen. > > Signed-off-by: Michael Wang <yun.wang@profitbricks.com> > --- > block/blk-core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 43b7d06..91223b2 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -2619,8 +2619,10 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes) > struct bio *bio = req->bio; > unsigned bio_bytes = min(bio->bi_iter.bi_size, nr_bytes); > > - if (bio_bytes == bio->bi_iter.bi_size) > + if (bio_bytes == bio->bi_iter.bi_size) { > req->bio = bio->bi_next; > + bio->bi_next = NULL; > + } > > req_bio_endio(req, bio, bio_bytes, error); > > -- > 2.5.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: NeilBrown <neilb@suse.com> To: Michael Wang <yun.wang@profitbricks.com>, "linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-block@vger.kernel.org, linux-raid@vger.kernel.org Cc: Jens Axboe <axboe@kernel.dk>, Shaohua Li <shli@kernel.org>, Jinpu Wang <jinpu.wang@profitbricks.com> Subject: Re: [RFC PATCH] blk: reset 'bi_next' when bio is done inside request Date: Tue, 04 Apr 2017 07:25:45 +1000 [thread overview] Message-ID: <877f31kwti.fsf@notabene.neil.brown.name> (raw) In-Reply-To: <9505ff12-7307-7dec-76b5-2a233a592634@profitbricks.com> [-- Attachment #1: Type: text/plain, Size: 1927 bytes --] On Mon, Apr 03 2017, Michael Wang wrote: > blk_attempt_plug_merge() try to merge bio into request and chain them > by 'bi_next', while after the bio is done inside request, we forgot to > reset the 'bi_next'. > > This lead into BUG while removing all the underlying devices from md-raid1, > the bio once go through: > > md_do_sync() > sync_request() > generic_make_request() This is a read request from the "first" device. > blk_queue_bio() > blk_attempt_plug_merge() > CHAINED HERE > > will keep chained and reused by: > > raid1d() > sync_request_write() > generic_make_request() This is a write request to some other device, isn't it? If sync_request_write() is using a bio that has already been used, it should call bio_reset() and fill in the details again. However I don't see how that would happen. Can you give specific details on the situation that triggers the bug? Thanks, NeilBrown > BUG_ON(bio->bi_next) > > After reset the 'bi_next' this can no longer happen. > > Signed-off-by: Michael Wang <yun.wang@profitbricks.com> > --- > block/blk-core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 43b7d06..91223b2 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -2619,8 +2619,10 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes) > struct bio *bio = req->bio; > unsigned bio_bytes = min(bio->bi_iter.bi_size, nr_bytes); > > - if (bio_bytes == bio->bi_iter.bi_size) > + if (bio_bytes == bio->bi_iter.bi_size) { > req->bio = bio->bi_next; > + bio->bi_next = NULL; > + } > > req_bio_endio(req, bio, bio_bytes, error); > > -- > 2.5.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --]
next prev parent reply other threads:[~2017-04-03 21:25 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-03 12:05 [RFC PATCH] blk: reset 'bi_next' when bio is done inside request Michael Wang 2017-04-03 21:25 ` NeilBrown [this message] 2017-04-03 21:25 ` NeilBrown 2017-04-04 8:13 ` Michael Wang 2017-04-04 9:37 ` NeilBrown 2017-04-04 9:37 ` NeilBrown 2017-04-04 10:23 ` Michael Wang 2017-04-04 12:24 ` Michael Wang 2017-04-04 12:48 ` Michael Wang 2017-04-04 21:52 ` NeilBrown 2017-04-04 21:52 ` NeilBrown
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=877f31kwti.fsf@notabene.neil.brown.name \ --to=neilb@suse.com \ --cc=axboe@kernel.dk \ --cc=jinpu.wang@profitbricks.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-raid@vger.kernel.org \ --cc=shli@kernel.org \ --cc=yun.wang@profitbricks.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.