Re: [RFC PATCH 1/3] block: add copy offload support

[Mikulas Patocka <mpatocka@redhat.com>]

On Thu, 3 Feb 2022, Bart Van Assche wrote:

> On 2/3/22 10:50, Mikulas Patocka wrote:
> > On Tue, 1 Feb 2022, Bart Van Assche wrote:
> > > On 2/1/22 10:32, Mikulas Patocka wrote:
> > > >    /**
> > > > + * blk_queue_max_copy_sectors - set maximum copy offload sectors for
> > > > the
> > > > queue
> > > > + * @q:  the request queue for the device
> > > > + * @size:  the maximum copy offload sectors
> > > > + */
> > > > +void blk_queue_max_copy_sectors(struct request_queue *q, unsigned int
> > > > size)
> > > > +{
> > > > +	q->limits.max_copy_sectors = size;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(blk_queue_max_copy_sectors);
> > > 
> > > Please either change the unit of 'size' into bytes or change its type into
> > > sector_t.
> > 
> > blk_queue_chunk_sectors, blk_queue_max_discard_sectors,
> > blk_queue_max_write_same_sectors, blk_queue_max_write_zeroes_sectors,
> > blk_queue_max_zone_append_sectors also have the unit of sectors and the
> > argument is "unsigned int". Should blk_queue_max_copy_sectors be
> > different?
> 
> As far as I know using the type sector_t for variables that represent a number
> of sectors is a widely followed convention:
> 
> $ git grep -w sector_t | wc -l
> 2575
> 
> I would appreciate it if that convention would be used consistently, even if
> that means modifying existing code.
> 
> Thanks,
> 
> Bart.

Changing the sector limit variables in struct queue_limits from unsigned 
int to sector_t would increase the size of the structure and its cache 
footprint.

And we can't send bios larger than 4GiB anyway because bi_size is 32-bit.

Jens, what do you think about it? Should the sectors limits be sector_t?

Mikulas