| 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
| ||
|
Message-ID: <20230628153518.xquaulfmevdaa6d4@green245>
Date: Wed, 28 Jun 2023 21:05:18 +0530
From: Nitesh Shetty <nj.shetty@...sung.com>
To: Damien Le Moal <dlemoal@...nel.org>
Cc: Jens Axboe <axboe@...nel.dk>, Jonathan Corbet <corbet@....net>,
Alasdair Kergon <agk@...hat.com>,
Mike Snitzer <snitzer@...nel.org>, dm-devel@...hat.com,
Keith Busch <kbusch@...nel.org>,
Christoph Hellwig <hch@....de>,
Sagi Grimberg <sagi@...mberg.me>,
Chaitanya Kulkarni <kch@...dia.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>,
martin.petersen@...cle.com, linux-scsi@...r.kernel.org,
willy@...radead.org, hare@...e.de, djwong@...nel.org,
bvanassche@....org, ming.lei@...hat.com, nitheshshetty@...il.com,
gost.dev@...sung.com, Kanchan Joshi <joshi.k@...sung.com>,
Anuj Gupta <anuj20.g@...sung.com>, linux-block@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-nvme@...ts.infradead.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v13 1/9] block: Introduce queue limits for copy-offload
support
On 23/06/28 03:40PM, Damien Le Moal wrote:
>On 6/28/23 03:36, Nitesh Shetty wrote:
>> Add device limits as sysfs entries,
>> - copy_offload (RW)
>> - copy_max_bytes (RW)
>> - copy_max_bytes_hw (RO)
>>
>> Above limits help to split the copy payload in block layer.
>> copy_offload: used for setting copy offload(1) or emulation(0).
>> copy_max_bytes: maximum total length of copy in single payload.
>> copy_max_bytes_hw: Reflects the device supported maximum limit.
>>
>> Reviewed-by: Hannes Reinecke <hare@...e.de>
>> Signed-off-by: Nitesh Shetty <nj.shetty@...sung.com>
>> Signed-off-by: Kanchan Joshi <joshi.k@...sung.com>
>> Signed-off-by: Anuj Gupta <anuj20.g@...sung.com>
>> ---
>> Documentation/ABI/stable/sysfs-block | 33 +++++++++++++++
>> block/blk-settings.c | 24 +++++++++++
>> block/blk-sysfs.c | 63 ++++++++++++++++++++++++++++
>> include/linux/blkdev.h | 12 ++++++
>> include/uapi/linux/fs.h | 3 ++
>> 5 files changed, 135 insertions(+)
>>
>> diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
>> index c57e5b7cb532..3c97303f658b 100644
>> --- a/Documentation/ABI/stable/sysfs-block
>> +++ b/Documentation/ABI/stable/sysfs-block
>> @@ -155,6 +155,39 @@ Description:
>> last zone of the device which may be smaller.
>>
>>
>> +What: /sys/block/<disk>/queue/copy_offload
>> +Date: June 2023
>> +Contact: linux-block@...r.kernel.org
>> +Description:
>> + [RW] When read, this file shows whether offloading copy to a
>> + device is enabled (1) or disabled (0). Writing '0' to this
>> + file will disable offloading copies for this device.
>> + Writing any '1' value will enable this feature. If the device
>> + does not support offloading, then writing 1, will result in an
>> + error.
>
>I am still not convinced that this one is really necessary. copy_max_bytes_hw !=
>0 indicates that the devices supports copy offload. And setting copy_max_bytes
>to 0 can be used to disable copy offload (which probably should be the default
>for now).
>
Agreed, we will do this in next iteration.
>> +
>> +
>> +What: /sys/block/<disk>/queue/copy_max_bytes
>> +Date: June 2023
>> +Contact: linux-block@...r.kernel.org
>> +Description:
>> + [RW] This is the maximum number of bytes that the block layer
>> + will allow for a copy request. This will is always smaller or
>
>will is -> is
>
acked
>> + equal to the maximum size allowed by the hardware, indicated by
>> + 'copy_max_bytes_hw'. An attempt to set a value higher than
>> + 'copy_max_bytes_hw' will truncate this to 'copy_max_bytes_hw'.
>> +
>> +
>> +What: /sys/block/<disk>/queue/copy_max_bytes_hw
>
>Nit: In keeping with the spirit of attributes like
>max_hw_sectors_kb/max_sectors_kb, I would call this one copy_max_hw_bytes.
>
acked, will update in next iteration.
>> +Date: June 2023
>> +Contact: linux-block@...r.kernel.org
>> +Description:
>> + [RO] This is the maximum number of bytes that the hardware
>> + will allow for single data copy request.
>> + A value of 0 means that the device does not support
>> + copy offload.
>> +
>> +
>> What: /sys/block/<disk>/queue/crypto/
>> Date: February 2022
>> Contact: linux-block@...r.kernel.org
>> diff --git a/block/blk-settings.c b/block/blk-settings.c
>> index 4dd59059b788..738cd3f21259 100644
>> --- a/block/blk-settings.c
>> +++ b/block/blk-settings.c
>> @@ -59,6 +59,8 @@ void blk_set_default_limits(struct queue_limits *lim)
>> lim->zoned = BLK_ZONED_NONE;
>> lim->zone_write_granularity = 0;
>> lim->dma_alignment = 511;
>> + lim->max_copy_sectors_hw = 0;
>> + lim->max_copy_sectors = 0;
>> }
>>
>> /**
>> @@ -82,6 +84,8 @@ void blk_set_stacking_limits(struct queue_limits *lim)
>> lim->max_dev_sectors = UINT_MAX;
>> lim->max_write_zeroes_sectors = UINT_MAX;
>> lim->max_zone_append_sectors = UINT_MAX;
>> + lim->max_copy_sectors_hw = UINT_MAX;
>> + lim->max_copy_sectors = UINT_MAX;
>> }
>> EXPORT_SYMBOL(blk_set_stacking_limits);
>>
>> @@ -183,6 +187,22 @@ void blk_queue_max_discard_sectors(struct request_queue *q,
>> }
>> EXPORT_SYMBOL(blk_queue_max_discard_sectors);
>>
>> +/**
>> + * blk_queue_max_copy_sectors_hw - set max sectors for a single copy payload
>> + * @q: the request queue for the device
>> + * @max_copy_sectors: maximum number of sectors to copy
>> + **/
>> +void blk_queue_max_copy_sectors_hw(struct request_queue *q,
>> + unsigned int max_copy_sectors)
>> +{
>> + if (max_copy_sectors > (COPY_MAX_BYTES >> SECTOR_SHIFT))
>> + max_copy_sectors = COPY_MAX_BYTES >> SECTOR_SHIFT;
>> +
>> + q->limits.max_copy_sectors_hw = max_copy_sectors;
>> + q->limits.max_copy_sectors = max_copy_sectors;
>> +}
>> +EXPORT_SYMBOL_GPL(blk_queue_max_copy_sectors_hw);
>> +
>> /**
>> * blk_queue_max_secure_erase_sectors - set max sectors for a secure erase
>> * @q: the request queue for the device
>> @@ -578,6 +598,10 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
>> t->max_segment_size = min_not_zero(t->max_segment_size,
>> b->max_segment_size);
>>
>> + t->max_copy_sectors = min(t->max_copy_sectors, b->max_copy_sectors);
>> + t->max_copy_sectors_hw = min(t->max_copy_sectors_hw,
>> + b->max_copy_sectors_hw);
>> +
>> t->misaligned |= b->misaligned;
>>
>> alignment = queue_limit_alignment_offset(b, start);
>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
>> index afc797fb0dfc..43551778d035 100644
>> --- a/block/blk-sysfs.c
>> +++ b/block/blk-sysfs.c
>> @@ -199,6 +199,62 @@ static ssize_t queue_discard_zeroes_data_show(struct request_queue *q, char *pag
>> return queue_var_show(0, page);
>> }
>>
>> +static ssize_t queue_copy_offload_show(struct request_queue *q, char *page)
>> +{
>> + return queue_var_show(blk_queue_copy(q), page);
>> +}
>> +
>> +static ssize_t queue_copy_offload_store(struct request_queue *q,
>> + const char *page, size_t count)
>> +{
>> + unsigned long copy_offload;
>> + ssize_t ret = queue_var_store(©_offload, page, count);
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (copy_offload && !q->limits.max_copy_sectors_hw)
>> + return -EINVAL;
>> +
>> + if (copy_offload)
>> + blk_queue_flag_set(QUEUE_FLAG_COPY, q);
>> + else
>> + blk_queue_flag_clear(QUEUE_FLAG_COPY, q);
>> +
>> + return count;
>> +}
>
>See above. I think we can drop this attribute.
>
acked
Thank you,
Nitesh Shetty
Powered by blists - more mailing lists