On Mon, May 20, 2024 at 03:50:14PM +0530, Nitesh Shetty wrote:
> Add device limits as sysfs entries,
> - copy_max_bytes (RW)
> - copy_max_hw_bytes (RO)
>
> Above limits help to split the copy payload in block layer.
> copy_max_bytes: maximum total length of copy in single payload.
> copy_max_hw_bytes: Reflects the device supported maximum limit.
That's a bit of a weird way to phrase the commit log as the queue_limits
are the main thing (and there are three of them as required for the
scheme to work). The sysfs attributes really are just an artifact.
> @@ -231,10 +237,11 @@ int blk_set_default_limits(struct queue_limits *lim)
> {
> /*
> * Most defaults are set by capping the bounds in blk_validate_limits,
> - * but max_user_discard_sectors is special and needs an explicit
> - * initialization to the max value here.
> + * but max_user_discard_sectors and max_user_copy_sectors are special
> + * and needs an explicit initialization to the max value here.
s/needs/need/
> +/*
> + * blk_queue_max_copy_hw_sectors - set max sectors for a single copy payload
> + * @q: the request queue for the device
> + * @max_copy_sectors: maximum number of sectors to copy
> + */
> +void blk_queue_max_copy_hw_sectors(struct request_queue *q,
> + unsigned int max_copy_sectors)
> +{
> + struct queue_limits *lim = &q->limits;
> +
> + if (max_copy_sectors > (BLK_COPY_MAX_BYTES >> SECTOR_SHIFT))
> + max_copy_sectors = BLK_COPY_MAX_BYTES >> SECTOR_SHIFT;
> +
> + lim->max_copy_hw_sectors = max_copy_sectors;
> + lim->max_copy_sectors =
> + min(max_copy_sectors, lim->max_user_copy_sectors);
> +}
> +EXPORT_SYMBOL_GPL(blk_queue_max_copy_hw_sectors);
Please don't add new blk_queue_* helpers, everything should go through
the atomic queue limits API now. Also capping the hardware limit
here looks odd.
> + if (max_copy_bytes & (queue_logical_block_size(q) - 1))
> + return -EINVAL;
This should probably go into blk_validate_limits and just round down.
Also most block limits are in kb. Not that I really know why we are
doing that, but is there a good reason to deviate from that scheme?
