Re: [PATCH] block: refactor rq_qos_wait()
From: Muchun Song
Date: Tue Oct 29 2024 - 03:31:25 EST
> On Oct 25, 2024, at 15:50, Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> +CC Tejun
>
> 在 2024/10/24 12:35, Muchun Song 写道:
>> When rq_qos_wait() is first introduced, it is easy to understand. But
>> with some bug fixes applied, it is not easy for newcomers to understand
>> the whole logic under those fixes. In this patch, rq_qos_wait() is
>> refactored and more comments are added for better understanding. There
>> are 4 points for the improvement:
>> 1) Use waitqueue_active() instead of wq_has_sleeper() to eliminate
>> unnecessary memory barrier in wq_has_sleeper() which is supposed
>> to be used in waker side. In this case, we do need the barrier.
>> So use the cheaper one to locklessly test for waiters on the queue.
>> 2) There is already a macro DEFINE_WAIT_FUNC() to declare a
>> wait_queue_entry with a specified waking function. But there is not
>> a counterpart for initializing one wait_queue_entry with a
>> specified waking function. So introducing init_wait_func() for
>> this, which also could be used elsewhere (like filemap.c). It can
>> be used in rq_qos_wait() to use default_wake_function() to wake up
>> waiters, which could remove ->task field from rq_qos_wait_data.
>
> I think it's better to cook point 2 as a seperate patch.
>
> Whether or not, this patch LGTM.
Either is OK for me. I can update this in v2.
> Reviewed-by: Yu Kuai <yukuai3@xxxxxxxxxx>
Thanks for your review.
Thanks,
Muchun