On Tue, May 02, 2017 at 01:42:24PM +1000, NeilBrown wrote:
> This patch converts bioset_create() and
> bioset_create_nobvec() to not create a workqueue so
> alloctions will never trigger punt_bios_to_rescuer(). It
> also introduces bioset_create_rescued() and
> bioset_create_nobvec_rescued() which preserve the old
> behaviour.
>
> All callers of bioset_create() and bioset_create_nobvec(),
> that are inside block device drivers, are converted to the
> _rescued() version.
The above comment about *_nobvec() need to be updated.
>
> biosets used by filesystems or other top-level users do not
> need rescuing as the bio can never be queued behind other
> bios. This includes fs_bio_set, blkdev_dio_pool,
> btrfs_bioset, xfs_ioend_bioset, drbd_md_io_bio_set,
> and one allocated by target_core_iblock.c.
>
> biosets used by md/raid to not need rescuing as
> their usage was recently audited to revised to never
> risk deadlock.
>
> It is hoped that most, if not all, of the remaining biosets
> can end up being the non-rescued version.
>
> Signed-off-by: NeilBrown <ne...@suse.com>
> ---
> block/bio.c | 12 ++++++++++--
> block/blk-core.c | 2 +-
> drivers/md/bcache/super.c | 6 ++++--
> drivers/md/dm-crypt.c | 2 +-
> drivers/md/dm-io.c | 2 +-
> drivers/md/dm.c | 5 +++--
> include/linux/bio.h | 1 +
> 7 files changed, 21 insertions(+), 9 deletions(-)
DRBD can stack on block device too, so I guess it might need
BIOSET_NEED_RESCUER?
>
> diff --git a/block/bio.c b/block/bio.c
> index 3f1286ed301e..257606e742e0 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -363,6 +363,8 @@ static void punt_bios_to_rescuer(struct bio_set *bs)
> struct bio_list punt, nopunt;
> struct bio *bio;
>
> + if (!WARN_ON_ONCE(!bs->rescue_workqueue))
> + return;
I guess the above check should have been the following?
if (WARN_ON_ONCE(!bs->rescue_workqueue))
return;
> /*
> * In order to guarantee forward progress we must punt only bios that
> * were allocated from this bio_set; otherwise, if there was a bio on
> @@ -474,7 +476,8 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int
> nr_iovecs,
>
> if (current->bio_list &&
> (!bio_list_empty(¤t->bio_list[0]) ||
> - !bio_list_empty(¤t->bio_list[1])))
> + !bio_list_empty(¤t->bio_list[1])) &&
> + bs->rescue_workqueue)
> gfp_mask &= ~__GFP_DIRECT_RECLAIM;
>
> p = mempool_alloc(bs->bio_pool, gfp_mask);
> @@ -1925,7 +1928,7 @@ EXPORT_SYMBOL(bioset_free);
> * bioset_create - Create a bio_set
> * @pool_size: Number of bio and bio_vecs to cache in the mempool
> * @front_pad: Number of bytes to allocate in front of the returned bio
> - * @flags: Flags to modify behavior, currently only %BIOSET_NEED_BVECS
> + * @flags: Flags to modify behavior, currently %BIOSET_NEED_BVECS and
> %BIOSET_NEED_RESCUER
> *
> * Description:
> * Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller
> @@ -1936,6 +1939,8 @@ EXPORT_SYMBOL(bioset_free);
> * or things will break badly.
> * If %BIOSET_NEED_BVECS is set in @flags, a separate pool will be
> allocated
> * for allocating iovecs. This pool is not needed e.g. for
> bio_clone_fast().
> + * If %BIOSET_NEED_RESCUER is set, workqueue is create which can be used
> to
> + * dispatch queued requests when the mempool runs out of space.
> *
> */
> struct bio_set *bioset_create(unsigned int pool_size,
> @@ -1971,6 +1976,9 @@ struct bio_set *bioset_create(unsigned int pool_size,
> goto bad;
> }
>
> + if (!(flags & BIOSET_NEED_RESCUER))
> + return bs;
> +
> bs->rescue_workqueue = alloc_workqueue("bioset", WQ_MEM_RECLAIM, 0);
> if (!bs->rescue_workqueue)
> goto bad;
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 3797753f4085..ae51f159a2ca 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -730,7 +730,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t
> gfp_mask, int node_id)
> if (q->id < 0)
> goto fail_q;
>
> - q->bio_split = bioset_create(BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS);
> + q->bio_split = bioset_create(BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS |
> BIOSET_NEED_RESCUER);
> if (!q->bio_split)
> goto fail_id;
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 8b9dae98cc7e..bb50991a82d3 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -786,7 +786,8 @@ static int bcache_device_init(struct bcache_device *d,
> unsigned block_size,
>
> minor *= BCACHE_MINORS;
>
> - if (!(d->bio_split = bioset_create(4, offsetof(struct bbio, bio),
> BIOSET_NEED_BVECS)) ||
> + if (!(d->bio_split = bioset_create(4, offsetof(struct bbio, bio),
> + BIOSET_NEED_BVECS |
> BIOSET_NEED_RESCUER)) ||
> !(d->disk = alloc_disk(BCACHE_MINORS))) {
> ida_simple_remove(&bcache_minor, minor);
> return -ENOMEM;
> @@ -1520,7 +1521,8 @@ struct cache_set *bch_cache_set_alloc(struct cache_sb
> *sb)
> sizeof(struct bbio) + sizeof(struct bio_vec) *
> bucket_pages(c))) ||
> !(c->fill_iter = mempool_create_kmalloc_pool(1, iter_size)) ||
> - !(c->bio_split = bioset_create(4, offsetof(struct bbio, bio),
> BIOSET_NEED_BVECS)) ||
> + !(c->bio_split = bioset_create(4, offsetof(struct bbio, bio),
> + BIOSET_NEED_BVECS |
> BIOSET_NEED_RESCUER)) ||
> !(c->uuids = alloc_bucket_pages(GFP_KERNEL, c)) ||
> !(c->moving_gc_wq = alloc_workqueue("bcache_gc",
> WQ_MEM_RECLAIM, 0)) ||
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 5a98c1eb87d9..87155eb7b39c 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -1936,7 +1936,7 @@ static int crypt_ctr(struct dm_target *ti, unsigned int
> argc, char **argv)
> goto bad;
> }
>
> - cc->bs = bioset_create(MIN_IOS, 0, BIOSET_NEED_BVECS);
> + cc->bs = bioset_create(MIN_IOS, 0, BIOSET_NEED_BVECS |
> BIOSET_NEED_RESCUER);
> if (!cc->bs) {
> ti->error = "Cannot allocate crypt bioset";
> goto bad;
> diff --git a/drivers/md/dm-io.c b/drivers/md/dm-io.c
> index 1d86d1d39d48..95b9ac98fe0c 100644
> --- a/drivers/md/dm-io.c
> +++ b/drivers/md/dm-io.c
> @@ -58,7 +58,7 @@ struct dm_io_client *dm_io_client_create(void)
> if (!client->pool)
> goto bad;
>
> - client->bios = bioset_create(min_ios, 0, BIOSET_NEED_BVECS);
> + client->bios = bioset_create(min_ios, 0, BIOSET_NEED_BVECS |
> BIOSET_NEED_RESCUER);
> if (!client->bios)
> goto bad;
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index e21a7d58c8ef..4f96556cb8c5 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1014,7 +1014,8 @@ static void flush_current_bio_list(struct blk_plug_cb
> *cb, bool from_schedule)
>
> while ((bio = bio_list_pop(&list))) {
> struct bio_set *bs = bio->bi_pool;
> - if (unlikely(!bs) || bs == fs_bio_set) {
> + if (unlikely(!bs) || bs == fs_bio_set ||
> + !bs->rescue_workqueue) {
> bio_list_add(¤t->bio_list[i], bio);
> continue;
> }
> @@ -2601,7 +2602,7 @@ struct dm_md_mempools *dm_alloc_md_mempools(struct
> mapped_device *md, unsigned t
> BUG();
> }
>
> - pools->bs = bioset_create(pool_size, front_pad, 0);
> + pools->bs = bioset_create(pool_size, front_pad, BIOSET_NEED_RESCUER);
> if (!pools->bs)
> goto out;
>
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 0975da6bebd9..40e5d7b62f29 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -376,6 +376,7 @@ static inline struct bio *bio_next_split(struct bio *bio,
> int sectors,
> extern struct bio_set *bioset_create(unsigned int, unsigned int, int flags);
> enum {
> BIOSET_NEED_BVECS = BIT(0),
> + BIOSET_NEED_RESCUER = BIT(1),
> };
> extern void bioset_free(struct bio_set *);
> extern mempool_t *biovec_create_pool(int pool_entries);
>
>
Thanks,
Ming
