linux-kernel - Re: [PATCH] block: refactor rq_qos

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <C50E9EB1-809B-4F69-86E1-96D307BF077C@linux.dev>
Date: Tue, 29 Oct 2024 15:30:08 +0800
From: Muchun Song <muchun.song@...ux.dev>
To: Yu Kuai <yukuai1@...weicloud.com>
Cc: Muchun Song <songmuchun@...edance.com>,
 mingo@...hat.com,
 peterz@...radead.org,
 chengming.zhou@...ux.dev,
 linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 tj@...nel.org,
 "yukuai (C)" <yukuai3@...wei.com>,
 axboe@...nel.dk
Subject: Re: [PATCH] block: refactor rq_qos_wait()



> On Oct 25, 2024, at 15:50, Yu Kuai <yukuai1@...weicloud.com> 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@...wei.com>

Thanks for your review.

Thanks,
Muchun