Re: [PATCH v13 3/9] block: add emulation for copy

Nitesh Shetty
2023-06-29T07:49:26Z

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

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

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