Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Fri, 22 Sep 2023 16:41:41 +0530
From:   Nitesh Shetty <nj.shetty@samsung.com>
To:     Jinyoung Choi <j-young.choi@samsung.com>
Cc:     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>,
        "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>
Subject: Re: [PATCH v16 03/12] block: add copy offload support
Message-ID: <20230922111141.GA14640@green245>
MIME-Version: 1.0
In-Reply-To: <20230922095650epcms2p8e25340eff5de01f8b3ce63ae81266881@epcms2p8>
User-Agent: Mutt/1.9.4 (2018-02-28)
Content-Type: multipart/mixed;
        boundary="----r74_f6S_OIQxXK41.4aq6v4zMY6akVoN.sMgyBemj3VfQZgP=_119e9_"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20230920080756.11919-4-nj.shetty@samsung.com>
        <20230920080756.11919-1-nj.shetty@samsung.com>
        <CGME20230920081447epcas5p144e631c5b8c72acf64d38b04d6c2c925@epcms2p8>
        <20230922095650epcms2p8e25340eff5de01f8b3ce63ae81266881@epcms2p8>
Precedence: bulk

------r74_f6S_OIQxXK41.4aq6v4zMY6akVoN.sMgyBemj3VfQZgP=_119e9_
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 8bit
Content-Disposition: inline

On Fri, Sep 22, 2023 at 06:56:50PM +0900, Jinyoung Choi wrote:
> > +/*
> > + * This must only be called once all bios have been issued so that the refcount
> > + * 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,
> > +? ? ? ? ? ? ? ? ? ? ? ? ? ? loff_t pos_out, size_t len,
> > +? ? ? ? ? ? ? ? ? ? ? ? ? ? void (*endio)(void *, int, ssize_t),
> > +? ? ? ? ? ? ? ? ? ? ? ? ? ? void *private, gfp_t gfp)
> > +{
> > +? ? ? ? struct blkdev_copy_io *cio;
> > +? ? ? ? struct blkdev_copy_offload_io *offload_io;
> > +? ? ? ? struct bio *src_bio, *dst_bio;
> > +? ? ? ? ssize_t rem, chunk, ret;
> > +? ? ? ? ssize_t max_copy_bytes = bdev_max_copy_sectors(bdev) << SECTOR_SHIFT;
> 
> wouldn't it be better to use size_t for variables that don't return?
> values such as chunk and max_copy_bytes may be defined as 'unsigned'.

Agree, we will keep ret as ssize_t and move others to size_t.
Acked for all other comments, will address them in next version.

Thank You,
Nitesh Shetty

------r74_f6S_OIQxXK41.4aq6v4zMY6akVoN.sMgyBemj3VfQZgP=_119e9_
Content-Type: text/plain; charset="utf-8"


------r74_f6S_OIQxXK41.4aq6v4zMY6akVoN.sMgyBemj3VfQZgP=_119e9_--