| Message ID | 1720cf81-6170-4cac-abf3-e19a4493653b@kernel.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: ensure that struct blk_mq_alloc_data is fully initialized | expand |
On 4/15/25 07:51, Jens Axboe wrote: > On x86, rep stos will be emitted to clear the the blk_mq_alloc_data > struct, as not all members are being initialied. Depending on the > type of CPU, this is a noticeable slowdown compared to just ensuring > that the struct is fully initialized when setup. > > Signed-off-by: Jens Axboe<axboe@kernel.dk> Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck
On 4/15/25 7:51 AM, Jens Axboe wrote: > On x86, rep stos will be emitted to clear the the blk_mq_alloc_data > struct, as not all members are being initialied. "Partial initialization" never happens in the C language when initializing a data structure. If a data structure is initialized, members that have not been specified are initialized to zero (the compiler is not required to initialize padding bytes). In other words, the description of this patch needs to be improved. Thanks, Bart.
On 4/15/25 9:50 AM, Bart Van Assche wrote: > On 4/15/25 7:51 AM, Jens Axboe wrote: >> On x86, rep stos will be emitted to clear the the blk_mq_alloc_data >> struct, as not all members are being initialied. > > "Partial initialization" never happens in the C language when > initializing a data structure. If a data structure is initialized, > members that have not been specified are initialized to zero (the > compiler is not required to initialize padding bytes). In other words, > the description of this patch needs to be improved. How is the description inaccurate? As not all members are being explicitly initialized, rep stos is emitted to do so.
On 4/15/25 8:59 AM, Jens Axboe wrote: > On 4/15/25 9:50 AM, Bart Van Assche wrote: >> On 4/15/25 7:51 AM, Jens Axboe wrote: >>> On x86, rep stos will be emitted to clear the the blk_mq_alloc_data >>> struct, as not all members are being initialied. >> >> "Partial initialization" never happens in the C language when >> initializing a data structure. If a data structure is initialized, >> members that have not been specified are initialized to zero (the >> compiler is not required to initialize padding bytes). In other words, >> the description of this patch needs to be improved. > > How is the description inaccurate? As not all members are being > explicitly initialized, rep stos is emitted to do so. Hi Jens, I think we agree if the word "explicit" would be added to the patch description. Thanks, Bart.
On 4/15/25 10:13 AM, Bart Van Assche wrote: > On 4/15/25 8:59 AM, Jens Axboe wrote: >> On 4/15/25 9:50 AM, Bart Van Assche wrote: >>> On 4/15/25 7:51 AM, Jens Axboe wrote: >>>> On x86, rep stos will be emitted to clear the the blk_mq_alloc_data >>>> struct, as not all members are being initialied. >>> >>> "Partial initialization" never happens in the C language when >>> initializing a data structure. If a data structure is initialized, >>> members that have not been specified are initialized to zero (the >>> compiler is not required to initialize padding bytes). In other words, >>> the description of this patch needs to be improved. >> >> How is the description inaccurate? As not all members are being >> explicitly initialized, rep stos is emitted to do so. > > Hi Jens, > > I think we agree if the word "explicit" would be added to the patch > description. Sure
diff --git a/block/blk-mq.c b/block/blk-mq.c index c2697db59109..9fb43b09a401 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -584,9 +584,13 @@ static struct request *blk_mq_rq_cache_fill(struct request_queue *q, struct blk_mq_alloc_data data = { .q = q, .flags = flags, + .shallow_depth = 0, .cmd_flags = opf, + .rq_flags = 0, .nr_tags = plug->nr_ios, .cached_rqs = &plug->cached_rqs, + .ctx = NULL, + .hctx = NULL }; struct request *rq; @@ -646,8 +650,13 @@ struct request *blk_mq_alloc_request(struct request_queue *q, blk_opf_t opf, struct blk_mq_alloc_data data = { .q = q, .flags = flags, + .shallow_depth = 0, .cmd_flags = opf, + .rq_flags = 0, .nr_tags = 1, + .cached_rqs = NULL, + .ctx = NULL, + .hctx = NULL }; int ret; @@ -675,8 +684,13 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q, struct blk_mq_alloc_data data = { .q = q, .flags = flags, + .shallow_depth = 0, .cmd_flags = opf, + .rq_flags = 0, .nr_tags = 1, + .cached_rqs = NULL, + .ctx = NULL, + .hctx = NULL }; u64 alloc_time_ns = 0; struct request *rq; @@ -2969,8 +2983,14 @@ static struct request *blk_mq_get_new_requests(struct request_queue *q, { struct blk_mq_alloc_data data = { .q = q, - .nr_tags = 1, + .flags = 0, + .shallow_depth = 0, .cmd_flags = bio->bi_opf, + .rq_flags = 0, + .nr_tags = 1, + .cached_rqs = NULL, + .ctx = NULL, + .hctx = NULL }; struct request *rq;
On x86, rep stos will be emitted to clear the the blk_mq_alloc_data struct, as not all members are being initialied. Depending on the type of CPU, this is a noticeable slowdown compared to just ensuring that the struct is fully initialized when setup. Signed-off-by: Jens Axboe <axboe@kernel.dk> ---