linux-kernel - Re: [PATCH v16 03/12] block: add copy offload support

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]

Date:   Fri, 22 Sep 2023 16:41:41 +0530
From:   Nitesh Shetty <nj.shetty@...sung.com>
To:     Jinyoung Choi <j-young.choi@...sung.com>
Cc:     Jens Axboe <axboe@...nel.dk>, Jonathan Corbet <corbet@....net>,
        Alasdair Kergon <agk@...hat.com>,
        Mike Snitzer <snitzer@...nel.org>,
        "dm-devel@...hat.com" <dm-devel@...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>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "nitheshshetty@...il.com" <nitheshshetty@...il.com>,
        "anuj1072538@...il.com" <anuj1072538@...il.com>,
        SSDR Gost Dev <gost.dev@...sung.com>,
        "mcgrof@...nel.org" <mcgrof@...nel.org>,
        Hannes Reinecke <hare@...e.de>,
        Anuj Gupta <anuj20.g@...sung.com>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v16 03/12] block: add copy offload support

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