On 23/10/2022 14:12, Ming Lei wrote:
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 8070b6c10e8d..260adeb2e455 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -402,6 +402,10 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
}
}
+ rq->__data_len = 0;
+ rq->__sector = (sector_t) -1;
+ rq->bio = rq->biotail = NULL;
+
return rq;
}
@@ -591,9 +595,6 @@ struct request *blk_mq_alloc_request(struct request_queue *q, blk_opf_t opf,
if (!rq)
goto out_queue_exit;
}
- rq->__data_len = 0;
- rq->__sector = (sector_t) -1;
- rq->bio = rq->biotail = NULL;
This patch looks not good, why do you switch to initialize the three fields
twice in fast path?
Can you please show me how these are initialized twice?
If there is a real concern with this then we go with my original idea,
which was to copy the init method of blk_mq_alloc_request() (in
blk_mq_alloc_request_hctx())
BTW, we know blk_mq_alloc_request_hctx() has big trouble, so please
avoid to extend it to other use cases.
Yeah, I know this, but sometimes we just need to allocate for a specific
HW queue...
For my usecase of interest, it should not impact if the cpumask of the
HW queue goes offline after selecting the cpu in
blk_mq_alloc_request_hctx(), so any race is ok ... I think.
However it should be still possible to make blk_mq_alloc_request_hctx()
more robust. How about using something like work_on_cpu_safe() to
allocate and execute the request with blk_mq_alloc_request() on a cpu
associated with the HW queue, such that we know the cpu is online and
stays online until we execute it? Or also extent to
work_on_cpumask_safe() variant, so that we don't need to try all cpus in
the mask (to see if online)?
Thanks,
John
