Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754028AbdDFSSN (ORCPT ); Thu, 6 Apr 2017 14:18:13 -0400 Received: from imap0.codethink.co.uk ([185.43.218.159]:55611 "EHLO imap0.codethink.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752059AbdDFSSE (ORCPT ); Thu, 6 Apr 2017 14:18:04 -0400 Message-ID: <1491502678.10415.40.camel@codethink.co.uk> Subject: Re: [PATCH 4.4 25/26] blk: Ensure users for current->bio_list can see the full list. From: Ben Hutchings To: Jack Wang Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, NeilBrown , Jens Axboe , Greg Kroah-Hartman Date: Thu, 06 Apr 2017 19:17:58 +0100 In-Reply-To: <20170406083605.968271891@linuxfoundation.org> References: <20170406083604.362636628@linuxfoundation.org> <20170406083605.968271891@linuxfoundation.org> Organization: Codethink Ltd. Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6095 Lines: 176 On Thu, 2017-04-06 at 10:38 +0200, Greg Kroah-Hartman wrote: > 4.4-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: NeilBrown > > commit f5fe1b51905df7cfe4fdfd85c5fb7bc5b71a094f upstream. > > Commit 79bd99596b73 ("blk: improve order of bio handling in generic_make_request()") > changed current->bio_list so that it did not contain *all* of the > queued bios, but only those submitted by the currently running > make_request_fn. > > There are two places which walk the list and requeue selected bios, > and others that check if the list is empty. These are no longer > correct. > > So redefine current->bio_list to point to an array of two lists, which > contain all queued bios, and adjust various code to test or walk both > lists. > > Signed-off-by: NeilBrown > Fixes: 79bd99596b73 ("blk: improve order of bio handling in generic_make_request()") > Signed-off-by: Jens Axboe > [jwang: backport to 4.4] > Signed-off-by: Jack Wang > Signed-off-by: Greg Kroah-Hartman > --- > block/bio.c | 12 +++++++++--- > block/blk-core.c | 31 +++++++++++++++++++------------ > drivers/md/raid1.c | 3 ++- > drivers/md/raid10.c | 3 ++- > 4 files changed, 32 insertions(+), 17 deletions(-) Why did you drop the changes in drivers/md/dm.c? Ben. > --- a/block/bio.c > +++ b/block/bio.c > @@ -373,10 +373,14 @@ static void punt_bios_to_rescuer(struct > bio_list_init(&punt); > bio_list_init(&nopunt); > > - while ((bio = bio_list_pop(current->bio_list))) > + while ((bio = bio_list_pop(¤t->bio_list[0]))) > bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio); > + current->bio_list[0] = nopunt; > > - *current->bio_list = nopunt; > + bio_list_init(&nopunt); > + while ((bio = bio_list_pop(¤t->bio_list[1]))) > + bio_list_add(bio->bi_pool == bs ? &punt : &nopunt, bio); > + current->bio_list[1] = nopunt; > > spin_lock(&bs->rescue_lock); > bio_list_merge(&bs->rescue_list, &punt); > @@ -464,7 +468,9 @@ struct bio *bio_alloc_bioset(gfp_t gfp_m > * we retry with the original gfp_flags. > */ > > - if (current->bio_list && !bio_list_empty(current->bio_list)) > + if (current->bio_list && > + (!bio_list_empty(¤t->bio_list[0]) || > + !bio_list_empty(¤t->bio_list[1]))) > gfp_mask &= ~__GFP_DIRECT_RECLAIM; > > p = mempool_alloc(bs->bio_pool, gfp_mask); > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -2021,7 +2021,14 @@ end_io: > */ > blk_qc_t generic_make_request(struct bio *bio) > { > - struct bio_list bio_list_on_stack; > + /* > + * bio_list_on_stack[0] contains bios submitted by the current > + * make_request_fn. > + * bio_list_on_stack[1] contains bios that were submitted before > + * the current make_request_fn, but that haven't been processed > + * yet. > + */ > + struct bio_list bio_list_on_stack[2]; > blk_qc_t ret = BLK_QC_T_NONE; > > if (!generic_make_request_checks(bio)) > @@ -2038,7 +2045,7 @@ blk_qc_t generic_make_request(struct bio > * should be added at the tail > */ > if (current->bio_list) { > - bio_list_add(current->bio_list, bio); > + bio_list_add(¤t->bio_list[0], bio); > goto out; > } > > @@ -2057,17 +2064,17 @@ blk_qc_t generic_make_request(struct bio > * bio_list, and call into ->make_request() again. > */ > BUG_ON(bio->bi_next); > - bio_list_init(&bio_list_on_stack); > - current->bio_list = &bio_list_on_stack; > + bio_list_init(&bio_list_on_stack[0]); > + current->bio_list = bio_list_on_stack; > do { > struct request_queue *q = bdev_get_queue(bio->bi_bdev); > > if (likely(blk_queue_enter(q, __GFP_DIRECT_RECLAIM) == 0)) { > - struct bio_list lower, same, hold; > + struct bio_list lower, same; > > /* Create a fresh bio_list for all subordinate requests */ > - hold = bio_list_on_stack; > - bio_list_init(&bio_list_on_stack); > + bio_list_on_stack[1] = bio_list_on_stack[0]; > + bio_list_init(&bio_list_on_stack[0]); > > ret = q->make_request_fn(q, bio); > > @@ -2077,19 +2084,19 @@ blk_qc_t generic_make_request(struct bio > */ > bio_list_init(&lower); > bio_list_init(&same); > - while ((bio = bio_list_pop(&bio_list_on_stack)) != NULL) > + while ((bio = bio_list_pop(&bio_list_on_stack[0])) != NULL) > if (q == bdev_get_queue(bio->bi_bdev)) > bio_list_add(&same, bio); > else > bio_list_add(&lower, bio); > /* now assemble so we handle the lowest level first */ > - bio_list_merge(&bio_list_on_stack, &lower); > - bio_list_merge(&bio_list_on_stack, &same); > - bio_list_merge(&bio_list_on_stack, &hold); > + bio_list_merge(&bio_list_on_stack[0], &lower); > + bio_list_merge(&bio_list_on_stack[0], &same); > + bio_list_merge(&bio_list_on_stack[0], &bio_list_on_stack[1]); > } else { > bio_io_error(bio); > } > - bio = bio_list_pop(current->bio_list); > + bio = bio_list_pop(&bio_list_on_stack[0]); > } while (bio); > current->bio_list = NULL; /* deactivate */ > > --- a/drivers/md/raid1.c > +++ b/drivers/md/raid1.c > @@ -877,7 +877,8 @@ static sector_t wait_barrier(struct r1co > ((conf->start_next_window < > conf->next_resync + RESYNC_SECTORS) && > current->bio_list && > - !bio_list_empty(current->bio_list))), > + (!bio_list_empty(¤t->bio_list[0]) || > + !bio_list_empty(¤t->bio_list[1])))), > conf->resync_lock); > conf->nr_waiting--; > } > --- a/drivers/md/raid10.c > +++ b/drivers/md/raid10.c > @@ -946,7 +946,8 @@ static void wait_barrier(struct r10conf > !conf->barrier || > (conf->nr_pending && > current->bio_list && > - !bio_list_empty(current->bio_list)), > + (!bio_list_empty(¤t->bio_list[0]) || > + !bio_list_empty(¤t->bio_list[1]))), > conf->resync_lock); > conf->nr_waiting--; > } > > > -- Ben Hutchings Software Developer, Codethink Ltd.