All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nitesh Shetty <nj.shetty@samsung.com>
To: Jinyoung Choi <j-young.choi@samsung.com>
Cc: "anuj1072538@gmail.com" <anuj1072538@gmail.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"dm-devel@redhat.com" <dm-devel@redhat.com>,
	Christoph Hellwig <hch@lst.de>, Alasdair Kergon <agk@redhat.com>,
	Sagi Grimberg <sagi@grimberg.me>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	SSDR Gost Dev <gost.dev@samsung.com>,
	"nitheshshetty@gmail.com" <nitheshshetty@gmail.com>,
	Chaitanya Kulkarni <kch@nvidia.com>,
	Anuj Gupta <anuj20.g@samsung.com>,
	Mike Snitzer <snitzer@kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@kernel.dk>,
	Christian Brauner <brauner@kernel.org>,
	"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"mcgrof@kernel.org" <mcgrof@kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>
Subject: Re: [dm-devel] [PATCH v16 03/12] block: add copy offload support
Date: Fri, 22 Sep 2023 16:41:41 +0530	[thread overview]
Message-ID: <20230922111141.GA14640@green245> (raw)
In-Reply-To: <20230922095650epcms2p8e25340eff5de01f8b3ce63ae81266881@epcms2p8>

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



[-- Attachment #3: Type: text/plain, Size: 98 bytes --]

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

WARNING: multiple messages have this Message-ID (diff)
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
Date: Fri, 22 Sep 2023 16:41:41 +0530	[thread overview]
Message-ID: <20230922111141.GA14640@green245> (raw)
In-Reply-To: <20230922095650epcms2p8e25340eff5de01f8b3ce63ae81266881@epcms2p8>

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



  reply	other threads:[~2023-09-23  8:13 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20230920081415epcas5p120fe03e79259894896a6bd04cf0845df@epcas5p1.samsung.com>
2023-09-20  8:07 ` [dm-devel] [PATCH v16 00/12] Implement copy offload support Nitesh Shetty
2023-09-20  8:07   ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 01/12] block: Introduce queue limits and sysfs for copy-offload support Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 02/12] Add infrastructure for copy offload in block and request layer Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 03/12] block: add copy offload support Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-22  9:56     ` [dm-devel] " Jinyoung Choi
2023-09-22  9:56       ` Jinyoung Choi
2023-09-22 11:11       ` Nitesh Shetty [this message]
2023-09-22 11:11         ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 04/12] block: add emulation for copy Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-22 13:08     ` [dm-devel] " Jinyoung Choi
2023-09-22 13:08       ` Jinyoung Choi
2023-09-26 10:07       ` [dm-devel] " Nitesh Jagadeesh Shetty
2023-09-26 10:07         ` Nitesh Jagadeesh Shetty
2023-10-18 10:08         ` Nitesh Jagadeesh Shetty
2023-10-18 10:08           ` Nitesh Jagadeesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 05/12] fs/read_write: Enable copy_file_range for block device Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 06/12] fs, block: copy_file_range for def_blk_ops for direct " Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 07/12] nvme: add copy offload support Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 08/12] nvmet: add copy command support for bdev and file ns Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-10-02  1:58     ` [dm-devel] " kernel test robot
2023-10-02  1:58       ` kernel test robot
2023-09-20  8:07   ` [dm-devel] [PATCH v16 09/12] dm: Add support for copy offload Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 10/12] dm: Enable copy offload for dm-linear target Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 11/12] null: Enable trace capability for null block Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20  8:07   ` [dm-devel] [PATCH v16 12/12] null_blk: add support for copy offload Nitesh Shetty
2023-09-20  8:07     ` Nitesh Shetty
2023-09-20 10:54     ` [dm-devel] " kernel test robot
2023-09-20 10:54       ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230922111141.GA14640@green245 \
    --to=nj.shetty@samsung.com \
    --cc=agk@redhat.com \
    --cc=anuj1072538@gmail.com \
    --cc=anuj20.g@samsung.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=corbet@lwn.net \
    --cc=dm-devel@redhat.com \
    --cc=gost.dev@samsung.com \
    --cc=hch@lst.de \
    --cc=j-young.choi@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=kch@nvidia.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mcgrof@kernel.org \
    --cc=nitheshshetty@gmail.com \
    --cc=sagi@grimberg.me \
    --cc=snitzer@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.