Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755499AbdDGIeI (ORCPT ); Fri, 7 Apr 2017 04:34:08 -0400 Received: from mail-qt0-f174.google.com ([209.85.216.174]:35638 "EHLO mail-qt0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754196AbdDGId7 (ORCPT ); Fri, 7 Apr 2017 04:33:59 -0400 MIME-Version: 1.0 In-Reply-To: <1491502678.10415.40.camel@codethink.co.uk> References: <20170406083604.362636628@linuxfoundation.org> <20170406083605.968271891@linuxfoundation.org> <1491502678.10415.40.camel@codethink.co.uk> From: Jinpu Wang Date: Fri, 7 Apr 2017 10:33:37 +0200 Message-ID: Subject: Re: [PATCH 4.4 25/26] blk: Ensure users for current->bio_list can see the full list. To: Ben Hutchings Cc: "linux-kernel@vger.kernel.org" , stable , NeilBrown , Jens Axboe , Greg Kroah-Hartman Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7653 Lines: 187 On Thu, Apr 6, 2017 at 8:17 PM, Ben Hutchings wrote: > 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. > > Hi Ben, Because the code snip doesn't exist in 4.4. -- Jack Wang Linux Kernel Developer