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

[Nitesh Shetty]

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@xxxxxxxxxxx>
> > Signed-off-by: Vincent Fu <vincent.fu@xxxxxxxxxxx>
> > Signed-off-by: Anuj Gupta <anuj20.g@xxxxxxxxxxx>
> > ---
> >  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