linux-kernel - Re: [PATCH 01/10] block: introduce BLK_FEAT_WRITE_ZEROES

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]

Message-ID: <3569a77f-1f38-4764-b1e3-d0075775c7bb@huaweicloud.com>
Date: Fri, 13 Jun 2025 11:15:41 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: Christoph Hellwig <hch@....de>, linux-fsdevel@...r.kernel.org,
 linux-ext4@...r.kernel.org, linux-block@...r.kernel.org,
 dm-devel@...ts.linux.dev, linux-nvme@...ts.infradead.org,
 linux-scsi@...r.kernel.org, linux-xfs@...r.kernel.org,
 linux-kernel@...r.kernel.org, tytso@....edu, john.g.garry@...cle.com,
 bmarzins@...hat.com, chaitanyak@...dia.com, shinichiro.kawasaki@....com,
 brauner@...nel.org, martin.petersen@...cle.com, yi.zhang@...wei.com,
 chengzhihao1@...wei.com, yukuai3@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH 01/10] block: introduce BLK_FEAT_WRITE_ZEROES_UNMAP to
 queue limits features

On 2025/6/12 23:03, Darrick J. Wong wrote:
> On Thu, Jun 12, 2025 at 07:20:45PM +0800, Zhang Yi wrote:
>> On 2025/6/12 12:47, Christoph Hellwig wrote:
>>> On Wed, Jun 11, 2025 at 03:31:21PM +0800, Zhang Yi wrote:
>>>>>> +/* supports unmap write zeroes command */
>>>>>> +#define BLK_FEAT_WRITE_ZEROES_UNMAP	((__force blk_features_t)(1u << 17))
>>>>>
>>>>>
>>>>> Should this be exposed through sysfs as a read-only value?
>>>>
>>>> Uh, are you suggesting adding another sysfs interface to expose
>>>> this feature?
>>>
>>> That was the idea.  Or do we have another way to report this capability?
>>>
>>
>> Exposing this feature looks useful, but I think adding a new interface
>> might be somewhat redundant, and it's also difficult to name the new
>> interface. What about extend this interface to include 3 types? When
>> read, it exposes the following:
>>
>>  - none     : the device doesn't support BLK_FEAT_WRITE_ZEROES_UNMAP.
>>  - enabled  : the device supports BLK_FEAT_WRITE_ZEROES_UNMAP, but the
>>               BLK_FLAG_WRITE_ZEROES_UNMAP_DISABLED is not set.
>>  - disabled : the device supports BLK_FEAT_WRITE_ZEROES_UNMAP, and the
>>               BLK_FLAG_WRITE_ZEROES_UNMAP_DISABLED is set.
>>
>> Users can write '0' and '1' to disable and enable this operation if it
>> is not 'none', thoughts?
> 
> Perhaps it should reuse the enumeration pattern elsewhere in sysfs?
> For example,
> 
> # cat /sys/block/sda/queue/scheduler
> none [mq-deadline]
> # echo none > /sys/block/sda/queue/scheduler
> # cat /sys/block/sda/queue/scheduler
> [none] mq-deadline
> 
> (Annoying that this seems to be opencoded wherever it appears...)
> 

Yeah, this solution looks good to me. However, we currently have only
two selections (none and unmap). What if we keep it as is and simply
hide this interface if BLK_FEAT_WRITE_ZEROES_UNMAP is not set, making
it visible only when the device supports this feature? Something like
below:

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index e918b2c93aed..204ee4d5f63f 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -747,6 +747,9 @@ static umode_t queue_attr_visible(struct kobject *kobj, struct attribute *attr,
             attr == &queue_max_active_zones_entry.attr) &&
            !blk_queue_is_zoned(q))
                return 0;
+       if (attr == &queue_write_zeroes_unmap_entry.attr &&
+           !(q->limits.features & BLK_FEAT_WRITE_ZEROES_UNMAP))
+               return 0;

        return attr->mode;
 }

Thanks,
Yi.