On 10/21/18 6:17 AM, Ondrej Zary wrote: > On Friday 19 October 2018 18:35:00 Jens Axboe wrote: >> On 10/19/18 3:10 AM, Ming Lei wrote: >>> On Thu, Oct 18, 2018 at 03:24:47PM -0600, Jens Axboe wrote: >>>> We're only setting up the bounce bio sets if we happen >>>> to need bouncing for regular HIGHMEM, not if we only need >>>> it for ISA devices. >>>> >>>> Reported-by: Ondrej Zary <[email protected]> >>>> Tested-by: Ondrej Zary <[email protected]> >>>> Signed-off-by: Jens Axboe <[email protected]> >>>> >>>> diff --git a/block/bounce.c b/block/bounce.c >>>> index b30071ac4ec6..1356a2f4aae2 100644 >>>> --- a/block/bounce.c >>>> +++ b/block/bounce.c >>>> @@ -31,6 +31,24 @@ >>>> static struct bio_set bounce_bio_set, bounce_bio_split; >>>> static mempool_t page_pool, isa_page_pool; >>>> >>>> +static void init_bounce_bioset(void) >>>> +{ >>>> + static bool bounce_bs_setup; >>>> + int ret; >>>> + >>>> + if (bounce_bs_setup) >>>> + return; >>>> + >>>> + ret = bioset_init(&bounce_bio_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); >>>> + BUG_ON(ret); >>>> + if (bioset_integrity_create(&bounce_bio_set, BIO_POOL_SIZE)) >>>> + BUG_ON(1); >>>> + >>>> + ret = bioset_init(&bounce_bio_split, BIO_POOL_SIZE, 0, 0); >>>> + BUG_ON(ret); >>>> + bounce_bs_setup = true; >>> >>> When initcall is run from do_basic_setup(), all CPUs and scheduler have >>> been up, >>> it is likely that init_emergency_pool() is run when one driver is calling >>> blk_queue_bounce_limit() to start init_emergency_isa_pool(). >>> >>> So the above code might be run twice. >> >> Good point, it's actually not even safe right now with the mempool >> setup. How about the below? >> >> >> diff --git a/block/bounce.c b/block/bounce.c >> index b30071ac4ec6..ec0d99995f5f 100644 >> --- a/block/bounce.c >> +++ b/block/bounce.c >> @@ -31,6 +31,24 @@ >> static struct bio_set bounce_bio_set, bounce_bio_split; >> static mempool_t page_pool, isa_page_pool; >> >> +static void init_bounce_bioset(void) >> +{ >> + static bool bounce_bs_setup; >> + int ret; >> + >> + if (bounce_bs_setup) >> + return; >> + >> + ret = bioset_init(&bounce_bio_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); >> + BUG_ON(ret); >> + if (bioset_integrity_create(&bounce_bio_set, BIO_POOL_SIZE)) >> + BUG_ON(1); >> + >> + ret = bioset_init(&bounce_bio_split, BIO_POOL_SIZE, 0, 0); >> + BUG_ON(ret); >> + bounce_bs_setup = true; >> +} >> + >> #if defined(CONFIG_HIGHMEM) >> static __init int init_emergency_pool(void) >> { >> @@ -44,14 +62,7 @@ static __init int init_emergency_pool(void) >> BUG_ON(ret); >> pr_info("pool size: %d pages\n", POOL_SIZE); >> >> - ret = bioset_init(&bounce_bio_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); >> - BUG_ON(ret); >> - if (bioset_integrity_create(&bounce_bio_set, BIO_POOL_SIZE)) >> - BUG_ON(1); >> - >> - ret = bioset_init(&bounce_bio_split, BIO_POOL_SIZE, 0, 0); >> - BUG_ON(ret); >> - >> + init_bounce_bioset(); >> return 0; >> } >> >> @@ -86,6 +97,8 @@ static void *mempool_alloc_pages_isa(gfp_t gfp_mask, void >> *data) >> return mempool_alloc_pages(gfp_mask | GFP_DMA, data); >> } >> >> +static DEFINE_MUTEX(isa_mutex); >> + >> /* >> * gets called "every" time someone init's a queue with BLK_BOUNCE_ISA >> * as the max address, so check if the pool has already been created. >> @@ -94,14 +107,20 @@ int init_emergency_isa_pool(void) >> { >> int ret; >> >> - if (mempool_initialized(&isa_page_pool)) >> + mutex_lock(&isa_mutex); >> + >> + if (mempool_initialized(&isa_page_pool)) { >> + mutex_unlock(&isa_mutex); >> return 0; >> + } >> >> ret = mempool_init(&isa_page_pool, ISA_POOL_SIZE, >> mempool_alloc_pages_isa, >> mempool_free_pages, (void *) 0); >> BUG_ON(ret); >> >> pr_info("isa pool size: %d pages\n", ISA_POOL_SIZE); >> + init_bounce_bioset(); >> + mutex_unlock(&isa_mutex); >> return 0; >> } >> > > Works for me.
Thanks Ondrej, I'm going to queue it up for 4.20 and mark it for stable. -- Jens Axboe
