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

Message-ID: <20240531101721.f3sclknsowyceszx@nj.shetty@samsung.com>
Date: Fri, 31 May 2024 10:17:21 +0000
From: Nitesh Shetty <nj.shetty@...sung.com>
To: Bart Van Assche <bvanassche@....org>
Cc: 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>, 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>, 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 30/05/24 10:11AM, Bart Van Assche wrote:
>On 5/30/24 00:16, Nitesh Shetty wrote:
>>+static inline bool blk_copy_offload_attempt_combine(struct request_queue *q,
>>+                         struct bio *bio)
>>+{
>>+    struct blk_plug *plug = current->plug;
>>+    struct request *rq;
>>+
>>+    if (!plug || rq_list_empty(plug->mq_list))
>>+        return false;
>>+
>>+    rq_list_for_each(&plug->mq_list, rq) {
>>+        if (rq->q == q) {
>>+            if (!blk_copy_offload_combine(rq, bio))
>>+                return true;
>>+            break;
>>+        }
>>+
>>+        /*
>>+         * Only keep iterating plug list for combines if we have multiple
>>+         * queues
>>+         */
>>+        if (!plug->multiple_queues)
>>+            break;
>>+    }
>>+    return false;
>>+}
>
>This new approach has the following two disadvantages:
>* Without plug, REQ_OP_COPY_SRC and REQ_OP_COPY_DST are not combined. These two
>  operation types are the only operation types for which not using a plug causes
>  an I/O failure.
>* A loop is required to combine the REQ_OP_COPY_SRC and REQ_OP_COPY_DST operations.
>
>Please switch to the approach Hannes suggested, namely bio chaining. Chaining
>REQ_OP_COPY_SRC and REQ_OP_COPY_DST bios before these are submitted eliminates the
>two disadvantages mentioned above.
>
Bart, Hannes,

I see the following challenges with bio-chained approach.
1. partitioned device:
	We need to add the code which iterates over all bios and adjusts
	the sectors offsets.
2. dm/stacked device:
	We need to make major changes in dm, such as allocating cloned
	bios, IO splits, IO offset mappings. All of which need to
	iterate over chained BIOs.

Overall with chained BIOs we need to add a special handling only for copy
to iterate over chained BIOs and do the same thing which is being done
for single BIO at present.
Or am I missing something here ?

Thank You,
Nitesh Shetty