linux-kernel - Re: [PATCH v3 1/5] wbt: don't show valid wbt_lat

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Sep 2022 21:01:41 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     Jan Kara <jack@...e.cz>, Yu Kuai <yukuai1@...weicloud.com>
Cc:     paolo.valente@...aro.org, axboe@...nel.dk,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        yi.zhang@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v3 1/5] wbt: don't show valid wbt_lat_usec in sysfs while
 wbt is disabled

Hi, Jan

在 2022/09/26 19:47, Jan Kara 写道:
> On Mon 26-09-22 18:25:18, Yu Kuai wrote:
>> Hi, Jan
>>
>> 在 2022/09/26 17:44, Jan Kara 写道:
>>> On Thu 22-09-22 19:35:54, Yu Kuai wrote:
>>>> Currently, if wbt is initialized and then disabled by
>>>> wbt_disable_default(), sysfs will still show valid wbt_lat_usec, which
>>>> will confuse users that wbt is still enabled.
>>>>
>>>> This patch shows wbt_lat_usec as zero and forbid to set it while wbt
>>>> is disabled.
>>>
>>> So I agree we should show 0 in wbt_lat_usec if wbt is disabled by
>>> wbt_disable_default(). But why do you forbid setting of wbt_lat_usec?
>>> IMHO if wbt_lat_usec is set, admin wants to turn on wbt so we should just
>>> update rwb->enable_state to WBT_STATE_ON_MANUAL.
>>
>> I was thinking that don't enable wbt if elevator is bfq. Since we know
>> that performance is bad, thus it doesn't make sense to me to do that,
>> and user might doesn't aware of the problem.
> 
> Yeah, I don't think it is a good idea (that is the reason why it is
> disabled by default) but in priciple I don't see a reason why we should
> block admin from enabling it.

I'll enable it in next version.

Thanks,
Kuai