Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: 8jaZBTanwCqy4mEbB/a3zGKcpVqfVH5bx6iG7Wq8QNK38OkBXLE7PCqQH2q3cT5HdQ8wDbUBNU
 KYm5fgTABi/muPEBYSpuEy4JdcSlcctHDNTV5gvcoj+codlgCJWN6TbODBcU7N768yUXrZY363
 IlgCEcK+J1Okid47UdncuWv157VXUU0Qj5YSaZbEw759vwsVNGXq/DxqWCG8DGiKnjCzdrEQGy
 olZRLhHFJ6UDPaE9X9Hv+e9qm/xCYixUZs7wbeZGghhNcUXfi5hDm/J7R1TFkyhSej136qe2NN
 dko=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     SelvaKumar S <selvakuma.s1@samsung.com>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
CC:     "kbusch@kernel.org" <kbusch@kernel.org>,
        "axboe@kernel.dk" <axboe@kernel.dk>, "hch@lst.de" <hch@lst.de>,
        "sagi@grimberg.me" <sagi@grimberg.me>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "dm-devel@redhat.com" <dm-devel@redhat.com>,
        "snitzer@redhat.com" <snitzer@redhat.com>,
        "selvajove@gmail.com" <selvajove@gmail.com>,
        "nj.shetty@samsung.com" <nj.shetty@samsung.com>,
        "joshi.k@samsung.com" <joshi.k@samsung.com>,
        "javier.gonz@samsung.com" <javier.gonz@samsung.com>
Subject: Re: [RFC PATCH v2 0/2] add simple copy support
Thread-Topic: [RFC PATCH v2 0/2] add simple copy support
Thread-Index: AQHWyiztcinYLb1afkCNvyh+SmI/oQ==
Date:   Fri, 4 Dec 2020 11:25:12 +0000
Message-ID: <CH2PR04MB6522F1188557C829285ED5E8E7F10@CH2PR04MB6522.namprd04.prod.outlook.com>
References: <CGME20201204094719epcas5p23b3c41223897de3840f92ae3c229cda5@epcas5p2.samsung.com>
 <20201204094659.12732-1-selvakuma.s1@samsung.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 365128f2-0c71-4214-0ced-08d8984743f0
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Dec 2020 11:25:12.1655
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: JyxWB2Oe3jjtYFH1Ut9EdnYdm1bmjcp5idVZeopuDqXjNy6AFjBfCcQHpLXzjjG5f4LmVEqa0MI+YVUXAQCUrA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR04MB6773
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/12/04 20:02, SelvaKumar S wrote:=0A=
> This patchset tries to add support for TP4065a ("Simple Copy Command"),=
=0A=
> v2020.05.04 ("Ratified")=0A=
> =0A=
> The Specification can be found in following link.=0A=
> https://nvmexpress.org/wp-content/uploads/NVM-Express-1.4-Ratified-TPs-1.=
zip=0A=
> =0A=
> This is an RFC. Looking forward for any feedbacks or other alternate=0A=
> designs for plumbing simple copy to IO stack.=0A=
> =0A=
> Simple copy command is a copy offloading operation and is  used to copy=
=0A=
> multiple contiguous ranges (source_ranges) of LBA's to a single destinati=
on=0A=
> LBA within the device reducing traffic between host and device.=0A=
> =0A=
> This implementation accepts destination, no of sources and arrays of=0A=
> source ranges from application and attach it as payload to the bio and=0A=
> submits to the device.=0A=
> =0A=
> Following limits are added to queue limits and are exposed in sysfs=0A=
> to userspace=0A=
> 	- *max_copy_sectors* limits the sum of all source_range length=0A=
> 	- *max_copy_nr_ranges* limits the number of source ranges=0A=
> 	- *max_copy_range_sectors* limit the maximum number of sectors=0A=
> 		that can constitute a single source range.=0A=
=0A=
Same comment as before. I think this is a good start, but for this to be re=
ally=0A=
useful to users and kernel components alike, this really needs copy emulati=
on=0A=
for drives that do not have a native copy feature, similarly to what write =
zeros=0A=
handling for instance: if the drive does not have a copy command (simple co=
py=0A=
for NVMe or XCOPY for scsi), then the block layer should issue read/write=
=0A=
commands to seamlessly execute the copy. Otherwise, this will only serve a =
small=0A=
niche for users and will not be optimal for FS and DM drivers that could be=
=0A=
simplified with a generic block layer copy functionality.=0A=
=0A=
This is my 10 cents though, others may differ about this.=0A=
=0A=
> =0A=
> Changes from v1:=0A=
> =0A=
> 1. Fix memory leak in __blkdev_issue_copy=0A=
> 2. Unmark blk_check_copy inline=0A=
> 3. Fix line break in blk_check_copy_eod=0A=
> 4. Remove p checks and made code more readable=0A=
> 5. Don't use bio_set_op_attrs and remove op and set=0A=
>    bi_opf directly=0A=
> 6. Use struct_size to calculate total_size=0A=
> 7. Fix partition remap of copy destination=0A=
> 8. Remove mcl,mssrl,msrc from nvme_ns=0A=
> 9. Initialize copy queue limits to 0 in nvme_config_copy=0A=
> 10. Remove return in QUEUE_FLAG_COPY check=0A=
> 11. Remove unused OCFS=0A=
> =0A=
> SelvaKumar S (2):=0A=
>   block: add simple copy support=0A=
>   nvme: add simple copy support=0A=
> =0A=
>  block/blk-core.c          |  94 ++++++++++++++++++++++++++---=0A=
>  block/blk-lib.c           | 123 ++++++++++++++++++++++++++++++++++++++=
=0A=
>  block/blk-merge.c         |   2 +=0A=
>  block/blk-settings.c      |  11 ++++=0A=
>  block/blk-sysfs.c         |  23 +++++++=0A=
>  block/blk-zoned.c         |   1 +=0A=
>  block/bounce.c            |   1 +=0A=
>  block/ioctl.c             |  43 +++++++++++++=0A=
>  drivers/nvme/host/core.c  |  87 +++++++++++++++++++++++++++=0A=
>  include/linux/bio.h       |   1 +=0A=
>  include/linux/blk_types.h |  15 +++++=0A=
>  include/linux/blkdev.h    |  15 +++++=0A=
>  include/linux/nvme.h      |  43 ++++++++++++-=0A=
>  include/uapi/linux/fs.h   |  13 ++++=0A=
>  14 files changed, 461 insertions(+), 11 deletions(-)=0A=
> =0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=