Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 28 Jun 2023 21:40:31 +0530
From:   Nitesh Shetty <nj.shetty@samsung.com>
To:     Damien Le Moal <dlemoal@kernel.org>
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,
        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, linux-scsi@vger.kernel.org,
        willy@infradead.org, hare@suse.de, djwong@kernel.org,
        bvanassche@acm.org, ming.lei@redhat.com, nitheshshetty@gmail.com,
        gost.dev@samsung.com, Vincent Fu <vincent.fu@samsung.com>,
        Anuj Gupta <anuj20.g@samsung.com>, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-nvme@lists.infradead.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v13 3/9] block: add emulation for copy
Message-ID: <20230628161031.snidi5qcpiffl2bx@green245>
MIME-Version: 1.0
In-Reply-To: <cdd190e3-d510-f4f3-46ee-3570f0317501@kernel.org>
User-Agent: NeoMutt/20171215
Content-Type: multipart/mixed;
        boundary="----PcSJ1RlEyPKfjXIiTsweH-7MCPtMf9v3nlMrb72LWO-fIHxy=_96484_"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20230627183629.26571-1-nj.shetty@samsung.com>
        <CGME20230627184020epcas5p13fdcea52edead5ffa3fae444f923439e@epcas5p1.samsung.com>
        <20230627183629.26571-4-nj.shetty@samsung.com>
        <cdd190e3-d510-f4f3-46ee-3570f0317501@kernel.org>
Precedence: bulk

------PcSJ1RlEyPKfjXIiTsweH-7MCPtMf9v3nlMrb72LWO-fIHxy=_96484_
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Disposition: inline

On 23/06/28 03:50PM, Damien Le Moal wrote:
>On 6/28/23 03:36, Nitesh Shetty wrote:
>> For the devices which does not support copy, copy emulation is added.
>> It is required for in-kernel users like fabrics, where file descriptor is
>> not available and hence they can't use copy_file_range.
>> Copy-emulation is implemented by reading from source into memory and
>> writing to the corresponding destination asynchronously.
>> Also emulation is used, if copy offload fails or partially completes.
>>
>> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
>> Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
>> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
>> ---
>>  block/blk-lib.c           | 183 +++++++++++++++++++++++++++++++++++++-
>>  block/blk-map.c           |   4 +-
>>  include/linux/blk_types.h |   5 ++
>>  include/linux/blkdev.h    |   3 +
>>  4 files changed, 192 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/blk-lib.c b/block/blk-lib.c
>> index 10c3eadd5bf6..09e0d5d51d03 100644
>> --- a/block/blk-lib.c
>> +++ b/block/blk-lib.c
>> @@ -234,6 +234,180 @@ static ssize_t __blkdev_copy_offload(
>>  	return blkdev_copy_wait_io_completion(cio);
>>  }
>>
>> +static void *blkdev_copy_alloc_buf(sector_t req_size, sector_t *alloc_size,
>> +		gfp_t gfp_mask)
>> +{
>> +	int min_size = PAGE_SIZE;
>> +	void *buf;
>> +
>> +	while (req_size >= min_size) {
>> +		buf = kvmalloc(req_size, gfp_mask);
>> +		if (buf) {
>> +			*alloc_size = req_size;
>> +			return buf;
>> +		}
>> +		/* retry half the requested size */
>
>Kind of obvious :)

Acked. will remove.

>
>> +		req_size >>= 1;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static void blkdev_copy_emulate_write_endio(struct bio *bio)
>> +{
>> +	struct copy_ctx *ctx = bio->bi_private;
>> +	struct cio *cio = ctx->cio;
>> +	sector_t clen;
>> +
>> +	if (bio->bi_status) {
>> +		clen = (bio->bi_iter.bi_sector << SECTOR_SHIFT) - cio->pos_out;
>> +		cio->comp_len = min_t(sector_t, clen, cio->comp_len);
>> +	}
>> +	kfree(bvec_virt(&bio->bi_io_vec[0]));
>> +	bio_map_kern_endio(bio);
>> +	kfree(ctx);
>> +	if (atomic_dec_and_test(&cio->refcount)) {
>> +		if (cio->endio) {
>> +			cio->endio(cio->private, cio->comp_len);
>> +			kfree(cio);
>> +		} else
>> +			blk_wake_io_task(cio->waiter);
>
>Curly brackets.
>
acked

>> +	}
>> +}
>> +
>> +static void blkdev_copy_emulate_read_endio(struct bio *read_bio)
>> +{
>> +	struct copy_ctx *ctx = read_bio->bi_private;
>> +	struct cio *cio = ctx->cio;
>> +	sector_t clen;
>> +
>> +	if (read_bio->bi_status) {
>> +		clen = (read_bio->bi_iter.bi_sector << SECTOR_SHIFT) -
>> +						cio->pos_in;
>> +		cio->comp_len = min_t(sector_t, clen, cio->comp_len);
>> +		kfree(bvec_virt(&read_bio->bi_io_vec[0]));
>> +		bio_map_kern_endio(read_bio);
>> +		kfree(ctx);
>> +
>> +		if (atomic_dec_and_test(&cio->refcount)) {
>> +			if (cio->endio) {
>> +				cio->endio(cio->private, cio->comp_len);
>> +				kfree(cio);
>> +			} else
>> +				blk_wake_io_task(cio->waiter);
>
>Same.

acked

>
>> +		}
>> +	}
>> +	schedule_work(&ctx->dispatch_work);
>
>ctx may have been freed above.

acked, will fix this.

>
>> +	kfree(read_bio);
>> +}
>> +
>> +static void blkdev_copy_dispatch_work(struct work_struct *work)
>> +{
>> +	struct copy_ctx *ctx = container_of(work, struct copy_ctx,
>> +			dispatch_work);
>
>Please align the argument, or even better: split the line after "=".


acked.

Thank you
Nitesh Shetty

------PcSJ1RlEyPKfjXIiTsweH-7MCPtMf9v3nlMrb72LWO-fIHxy=_96484_
Content-Type: text/plain; charset="utf-8"


------PcSJ1RlEyPKfjXIiTsweH-7MCPtMf9v3nlMrb72LWO-fIHxy=_96484_--