Re: [PATCH] blk-mq: clear stale inactive flag on hctx reuse

[Jens Axboe <axboe@kernel.dk>]

On 10/21/26 9:18 AM, Bart Van Assche wrote:
> On 10/20/26 7:39 AM, Jens Axboe wrote:
>> On 10/20/26 8:13 AM, Bart Van Assche wrote:
>>> On 10/20/26 5:51 AM, Nirmoy Das wrote:
>>>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>>>> index 025a799f35b924d..00b75eb95dd943a 100644
>>>> --- a/block/blk-mq.c
>>>> +++ b/block/blk-mq.c
>>>> @@ -4002,6 +4002,7 @@ static int blk_mq_init_hctx(struct request_queue *q,
>>>>            goto fail;
>>>>          hctx->queue_num = hctx_idx;
>>>> +    clear_bit(BLK_MQ_S_INACTIVE, &hctx->state);
>>>>          hctx->tags = set->tags[hctx_idx];
>>>
>>> Why clear_bit() instead of __clear_bit()?
>>
>> Since this isn't a hot path at all, going into details like
>> that does more harm than good. clear_bit() is totally fine
>> and honestly preferred.
> 
> Although I'm fine with the patch as-is, I want to clarify that
> the concern behind my reply is code readability rather than a
> micro-optimization. No other context should be accessing a hctx while
> blk_mq_init_hctx() is in progress. Using clear_bit() instead of
> __clear_bit() might make readers of this code wonder whether any other
> context might be accessing the hctx concurrently with the
> blk_mq_init_hctx() call.

I know that was your intent, but it just reads the opposite to
me. Whenever someone uses the __ variants it should only be
for optimization purposes, and preferably with a comment
explaining why it's safe. Unless there's a reason to do so,
using the normal variants is just preferable to me.

-- 
Jens Axboe