Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Mime-Version: 1.0
Subject: RE: [PATCH v16 03/12] block: add copy offload support
Reply-To: j-young.choi@samsung.com
Sender: Jinyoung Choi <j-young.choi@samsung.com>
From:   Jinyoung Choi <j-young.choi@samsung.com>
To:     Nitesh Jagadeesh Shetty <nj.shetty@samsung.com>,
        Jens Axboe <axboe@kernel.dk>, Jonathan Corbet <corbet@lwn.net>,
        Alasdair Kergon <agk@redhat.com>,
        Mike Snitzer <snitzer@kernel.org>,
        "dm-devel@redhat.com" <dm-devel@redhat.com>,
        Keith Busch <kbusch@kernel.org>,
        Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        Chaitanya Kulkarni <kch@nvidia.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <brauner@kernel.org>
CC:     "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "nitheshshetty@gmail.com" <nitheshshetty@gmail.com>,
        "anuj1072538@gmail.com" <anuj1072538@gmail.com>,
        SSDR Gost Dev <gost.dev@samsung.com>,
        "mcgrof@kernel.org" <mcgrof@kernel.org>,
        Hannes Reinecke <hare@suse.de>,
        Anuj Gupta <anuj20.g@samsung.com>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
In-Reply-To: <20230920080756.11919-4-nj.shetty@samsung.com>
Message-ID: <20230922095650epcms2p8e25340eff5de01f8b3ce63ae81266881@epcms2p8>
Date:   Fri, 22 Sep 2023 18:56:50 +0900
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <20230920080756.11919-4-nj.shetty@samsung.com>
        <20230920080756.11919-1-nj.shetty@samsung.com>
        <CGME20230920081447epcas5p144e631c5b8c72acf64d38b04d6c2c925@epcms2p8>
Precedence: bulk

> +/*
> + * This must only be called once all bios have been issued so that the r=
efcount
> + * can only decrease. This just waits for all bios to complete.
> + * Returns the length of bytes copied or error
> + */
> +static ssize_t blkdev_copy_wait_io_completion(struct blkdev_copy_io *cio=
)

Hi, Nitesh,

don't functions waiting for completion usually set their names to 'wait_for=
_completion_'?
(e.g. blkdev_copy_wait_for_completion_io)


> +ssize_t blkdev_copy_offload(struct block_device *bdev, loff_t pos_in,
> +=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20loff_t=20pos_out,=20s=
ize_t=20len,=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
void=20(*endio)(void=20*,=20int,=20ssize_t),=0D=0A>=20+=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20=C2=A0=20=C2=A0=20void=20*private,=20gfp_t=20gfp)=0D=0A>=20+=
=7B=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20struct=20blkdev_copy_io=
=20*cio;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20struct=20blkdev_copy=
_offload_io=20*offload_io;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20st=
ruct=20bio=20*src_bio,=20*dst_bio;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20ssize_t=20rem,=20chunk,=20ret;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20ssize_t=20max_copy_bytes=20=3D=20bdev_max_copy_sectors(bdev)=20<<=
=20SECTOR_SHIFT;=0D=0A=0D=0Awouldn't=20it=20be=20better=20to=20use=20size_t=
=20for=20variables=20that=20don't=20return?=0D=0Avalues=20such=20as=20chunk=
=20and=20max_copy_bytes=20may=20be=20defined=20as=20'unsigned'.=0D=0A=0D=0A=
>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20struct=20blk_plug=20plug;=0D=0A>=
=20+=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20if=20(=21max_copy_bytes)=
=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20return=20-EOPNOTSUPP;=0D=0A>=20+=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20ret=20=3D=20blkdev_copy_sanity_check(bdev,=20pos_in,=20bdev,=
=20pos_out,=20len);=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20if=20(ret=
)=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20return=20ret;=0D=0A>=20+=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20cio=20=3D=20kzalloc(sizeof(*cio),=20GFP_KERNEL);=0D=0A>=20+=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20if=20(=21cio)=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20return=20-ENOMEM;=0D=0A>=20=
+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20atomic_set(&cio->refcount,=201);=0D=0A=
>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20cio->waiter=20=3D=20current;=0D=0A=
>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20cio->endio=20=3D=20endio;=0D=0A>=
=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20cio->private=20=3D=20private;=0D=0A=
>=20+=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20/*=0D=0A>=20+=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0*=20If=20there=20is=20a=20error,=20copied=
=20will=20be=20set=20to=20least=20successfully=0D=0A>=20+=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0*=20completed=20copied=20length=0D=0A>=20+=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0*/=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20cio->copied=20=3D=20len;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20for=20(rem=20=3D=20len;=20rem=20>=200;=20rem=20-=3D=20chunk)=20=7B=0D=
=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20chunk=20=3D=20min(rem,=20max_copy_bytes);=0D=0A>=20+=0D=0A>=20+=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20offload_i=
o=20=3D=20kzalloc(sizeof(*offload_io),=20GFP_KERNEL);=0D=0A>=20+=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20if=20(=21off=
load_io)=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20goto=20err_free_cio;=
=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20offload_io->cio=20=3D=20cio;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20/*=0D=0A>=20+=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0*=20For=20pa=
rtial=20completion,=20we=20use=20offload_io->offset=20to=20truncate=0D=0A>=
=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0*=20successful=20copy=20length=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0*/=0D=0A>=20+=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20offload_i=
o->offset=20=3D=20len=20-=20rem;=0D=0A>=20+=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20src_bio=20=3D=20bio_allo=
c(bdev,=200,=20REQ_OP_COPY_SRC,=20gfp);=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20if=20(=21src_bio)=0D=0A>=20=
+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20goto=20err_free_offload_io;=0D=0A>=20+=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20src=
_bio->bi_iter.bi_size=20=3D=20chunk;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20src_bio->bi_iter.bi_sector=20=
=3D=20pos_in=20>>=20SECTOR_SHIFT;=0D=0A>=20+=0D=0A>=20+=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20blk_start_plug(&plug)=
;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20dst_bio=20=3D=20blk_next_bio(src_bio,=20bdev,=200,=20REQ_OP_COPY_D=
ST,=20gfp);=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20if=20(=21dst_bio)=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=
=C2=A0=20goto=20err_free_src_bio;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20dst_bio->bi_iter.bi_size=20=3D=20=
chunk;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20dst_bio->bi_iter.bi_sector=20=3D=20pos_out=20>>=20SECTOR_SHI=
FT;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20dst_bio->bi_end_io=20=3D=20blkdev_copy_offload_dst_endio;=0D=0A=
>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20dst_bio->bi_private=20=3D=20offload_io;=0D=0A>=20+=0D=0A>=20+=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20atomic_inc(&=
cio->refcount);=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20=C2=A0=20submit_bio(dst_bio);=0D=0A>=20+=C2=A0=20=C2=A0=20=
=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20blk_finish_plug(&plug=
);=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20pos_in=20+=3D=20chunk;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20pos_out=20+=3D=20chunk;=0D=0A>=20=
+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=7D=0D=0A>=20+=0D=0A>=20+=C2=A0=20=C2=
=A0=20=C2=A0=20=C2=A0=20if=20(atomic_dec_and_test(&cio->refcount))=0D=0A>=
=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20blkdev_copy_endio(cio);=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20if=
=20(cio->endio)=0D=0A=0D=0AIsn't=20it=20a=20problem=20if=20the=20memory=20o=
f=20cio=20is=20released=20in=20blkdev_copy_endio()?=0D=0AIt=20is=20unlikely=
=20to=20occur=20if=20there=20is=20an=20inflight=20i/o=20earlier,=0D=0Ait=20=
would=20be=20nice=20to=20modify=20considering=20code=20flow.=0D=0A=0D=0A=0D=
=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=
=A0=20return=20-EIOCBQUEUED;=0D=0A>=20+=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20return=20blkdev_copy_wait_io_completion(cio);=0D=0A>=20+=0D=0A>=
=20+err_free_src_bio:=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20bio_put=
(src_bio);=0D=0A>=20+err_free_offload_io:=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=
=A0=20=C2=A0=20kfree(offload_io);=0D=0A>=20+err_free_cio:=0D=0A>=20+=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20cio->copied=20=3D=20min_t(ssize_t,=20cio->cop=
ied,=20(len=20-=20rem));=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20cio-=
>status=20=3D=20-ENOMEM;=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20if=
=20(rem=20=3D=3D=20len)=20=7B=0D=0A>=20+=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=
=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20kfree(cio);=0D=0A>=20+=C2=A0=20=C2=
=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20=C2=A0=20return=20cio->s=
tatus;=0D=0A=0D=0Aisn't=20it=20a=20problem=20if=20the=20memory=20of=20cio=
=20is=20released?=0D=0A=0D=0ABest=20Regards,=0D=0AJinyoung.=0D=0A=0D=0A