linux-kernel - Re: [PATCH v20 02/12] Add infrastructure for copy offload in block and request layer.

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]

Date: Mon, 24 Jun 2024 16:14:07 +0530
From: Nitesh Shetty <nj.shetty@...sung.com>
To: Christoph Hellwig <hch@....de>
Cc: Bart Van Assche <bvanassche@....org>, Damien Le Moal
	<dlemoal@...nel.org>, Jens Axboe <axboe@...nel.dk>, Jonathan Corbet
	<corbet@....net>, Alasdair Kergon <agk@...hat.com>, Mike Snitzer
	<snitzer@...nel.org>, Mikulas Patocka <mpatocka@...hat.com>, Keith Busch
	<kbusch@...nel.org>, Sagi Grimberg <sagi@...mberg.me>, Chaitanya Kulkarni
	<kch@...dia.com>, Alexander Viro <viro@...iv.linux.org.uk>, Christian
	Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
	martin.petersen@...cle.com, david@...morbit.com, hare@...e.de,
	damien.lemoal@...nsource.wdc.com, anuj20.g@...sung.com, joshi.k@...sung.com,
	nitheshshetty@...il.com, gost.dev@...sung.com, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	dm-devel@...ts.linux.dev, linux-nvme@...ts.infradead.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v20 02/12] Add infrastructure for copy offload in block
 and request layer.

On 05/06/24 10:20AM, Christoph Hellwig wrote:
>On Tue, Jun 04, 2024 at 04:44:34AM -0700, Bart Van Assche wrote:
>> On 6/3/24 21:40, Christoph Hellwig wrote:
>>> There is no requirement to process them synchronously, there is just
>>> a requirement to preserve the order.  Note that my suggestion a few
>>> arounds ago also included a copy id to match them up.  If we don't
>>> need that I'm happy to leave it away.  If need it it to make stacking
>>> drivers' lifes easier that suggestion still stands.
>>
>> Including an ID in REQ_OP_COPY_DST and REQ_OP_COPY_SRC operations sounds
>> much better to me than abusing the merge infrastructure for combining
>> these two operations into a single request. With the ID-based approach
>> stacking drivers are allowed to process copy bios asynchronously and it
>> is no longer necessary to activate merging for copy operations if
>> merging is disabled (QUEUE_FLAG_NOMERGES).
>
>Again, we can decided on QUEUE_FLAG_NOMERGES per request type.  In fact
>I think we should not use it for discards as that just like copy
>is a very different kind of "merge".
>
>I'm in fact much more happy about avoiding the copy_id IFF we can.  It
>it a fair amout of extra overhead, so we should only add it if there
>is a real need for it

Christoph, Martin, Bart, Hannes, Damien

We have iterated over couple of designs for copy offload, but ended up
with changing them owing to some drawbacks. I would like to take your
opinion on how we can plumb copy offload in block and dm layer.

For reference, I have listed the approaches we have taken in the past.

a. Token/payload based approach:
1. Here we allocate a buffer/payload.
2. First source BIO is sent along with the buffer.
3. Once the buffer reaches driver, it is filled with the source LBA
and length and namespace info. And the request is completed.
4. Then destination BIO is sent with same buffer.
5. Once the buffer reaches driver, it retrieves the source information from
the BIO and forms a copy command and sends it down to device.

We received feedback that putting anything inside payload which is not
data, is not a good idea[1].

b. Plug based approach:
1. We take a plug.
2. Send a destination BIO, this forms a copy request and waits for source BIO
to arrive.
3. Send a source BIO, this merges with the copy request which was formed
by destination BIO.
4. We release the plug, then copy request reaches driver layer and forms
a copy command.

This approach won't work with stacked devices which has asynchronous
submission.
Overall taking plug and merging BIOs received not so good feedback from
community.

c. List/ctx based approach:
A new member is added to bio, bio_copy_ctx, which will a union with
bi_integrity. Idea is once a copy bio reaches blk_mq_submit_bio, it will
add the bio to this list.
1. Send the destination BIO, once this reaches blk_mq_submit_bio, this
will add the destination BIO to the list inside bi_copy_ctx and return
without forming any request.
2. Send source BIO, once this reaches blk_mq_submit_bio, this will
retrieve the destination BIO from bi_copy_ctx and form a request with
destination BIO and source BIO. After this request will be sent to
driver.

This work is still in POC phase[2]. But this approach makes lifetime
management of BIO complicated, especially during failure cases.

Thank You,
Nitesh Shetty

[1] https://lore.kernel.org/linux-block/20230605121732.28468-1-nj.shetty@samsung.com/
[2] https://github.com/SamsungDS/linux/tree/feat/co/v21/ctx