Re: [PATCH 1/2] block: bio: ensure newly added bio flags don't override BVEC_POOL_IDX

[Hannes Reinecke <hare@suse.de>]

On 3/21/19 1:30 PM, Johannes Thumshirn wrote:
> With the introduction of BIO_ALLOCED we've used up all available bits in
> bio::bi_flags.
> 
> Make sure no-one adds a new one and thus overrides the BVEC_POOL_IDX
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>   block/bio.c               |  8 ++++++++
>   include/linux/blk_types.h | 31 +++++++++++++++++--------------
>   2 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 4db1008309ed..8c689aed46a0 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -2126,12 +2126,20 @@ static void __init biovec_init_slabs(void)
>   	}
>   }
>   
> +static inline void bio_flag_buildtime_check(void)
> +{
> +	BUILD_BUG_ON(BIO_FLAG_LAST > BVEC_POOL_OFFSET);
> +}
> +
>   static int __init init_bio(void)
>   {
>   	bio_slab_max = 2;
>   	bio_slab_nr = 0;
>   	bio_slabs = kcalloc(bio_slab_max, sizeof(struct bio_slab),
>   			    GFP_KERNEL);
> +
> +	bio_flag_buildtime_check();
> +
>   	if (!bio_slabs)
>   		panic("bio: can't allocate bios\n");
>   
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index d66bf5f32610..0273bad71a96 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -215,20 +215,23 @@ struct bio {
>   /*
>    * bio flags
>    */
> -#define BIO_SEG_VALID	1	/* bi_phys_segments valid */
> -#define BIO_CLONED	2	/* doesn't own data */
> -#define BIO_BOUNCED	3	/* bio is a bounce bio */
> -#define BIO_USER_MAPPED 4	/* contains user pages */
> -#define BIO_NULL_MAPPED 5	/* contains invalid user pages */
> -#define BIO_QUIET	6	/* Make BIO Quiet */
> -#define BIO_CHAIN	7	/* chained bio, ->bi_remaining in effect */
> -#define BIO_REFFED	8	/* bio has elevated ->bi_cnt */
> -#define BIO_THROTTLED	9	/* This bio has already been subjected to
> -				 * throttling rules. Don't do it again. */
> -#define BIO_TRACE_COMPLETION 10	/* bio_endio() should trace the final completion
> -				 * of this bio. */
> -#define BIO_QUEUE_ENTERED 11	/* can use blk_queue_enter_live() */
> -#define BIO_TRACKED 12		/* set if bio goes through the rq_qos path */
> +enum {
> +	BIO_SEG_VALID =	1,		/* bi_phys_segments valid */
> +	BIO_CLONED = 2,			/* doesn't own data */
> +	BIO_BOUNCED = 3,		/* bio is a bounce bio */
> +	BIO_USER_MAPPED = 4,		/* contains user pages */
> +	BIO_NULL_MAPPED = 5,		/* contains invalid user pages */
> +	BIO_QUIET = 6,			/* Make BIO Quiet */
> +	BIO_CHAIN = 7,			/* chained bio, ->bi_remaining in effect */
> +	BIO_REFFED = 8,			/* bio has elevated ->bi_cnt */
> +	BIO_THROTTLED =	9,		/* This bio has already been subjected to
> +					 * throttling rules. Don't do it again. */
> +	BIO_TRACE_COMPLETION = 10,	/* bio_endio() should trace the final completion
> +					 * of this bio. */
> +	BIO_QUEUE_ENTERED = 11,		/* can use blk_queue_enter_live() */
> +	BIO_TRACKED = 12,		/* set if bio goes through the rq_qos path */
> +	BIO_FLAG_LAST
> +};
>   
>   /* See BVEC_POOL_OFFSET below before adding new flags */
>   
> 
Where's the point of the enum if all values are initialized?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)