Re: [PATCH] scsi_debug: Make max_sectors configurable

[John Garry <john.g.garry@oracle.com>]

On 15/05/2025 18:35, Bart Van Assche wrote:
> Make the SCSI debug host parameter max_sectors configurable to make it
> easier to trigger request splitting in the block layer.

Obviously we can do that via sysfs with the max_sectors file.

Is that really not good enough?

> If this parameter
> is not set then the following SCSI core code will set max sectors to 1024:
> 
> 	if (sht->max_sectors)
> 		shost->max_sectors = sht->max_sectors;
> 	else
> 		shost->max_sectors = SCSI_DEFAULT_MAX_SECTORS;
> 
> Cc: John Garry <john.g.garry@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/scsi/scsi_debug.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index aef33d1e346a..101a96530b11 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -913,6 +913,7 @@ static int sdebug_host_max_queue;	/* per host */
>   static int sdebug_lowest_aligned = DEF_LOWEST_ALIGNED;
>   static int sdebug_max_luns = DEF_MAX_LUNS;
>   static int sdebug_max_queue = SDEBUG_CANQUEUE;	/* per submit queue */
> +static unsigned int sdebug_max_sectors;
>   static unsigned int sdebug_medium_error_start = OPT_MEDIUM_ERR_ADDR;
>   static int sdebug_medium_error_count = OPT_MEDIUM_ERR_NUM;
>   static int sdebug_ndelay = DEF_NDELAY;	/* if > 0 then unit is nanoseconds */
> @@ -7314,6 +7315,7 @@ module_param_named(lowest_aligned, sdebug_lowest_aligned, int, S_IRUGO);
>   module_param_named(lun_format, sdebug_lun_am_i, int, S_IRUGO | S_IWUSR);
>   module_param_named(max_luns, sdebug_max_luns, int, S_IRUGO | S_IWUSR);
>   module_param_named(max_queue, sdebug_max_queue, int, S_IRUGO | S_IWUSR);
> +module_param_named(max_sectors, sdebug_max_sectors, uint, 0444);
>   module_param_named(medium_error_count, sdebug_medium_error_count, int,
>   		   S_IRUGO | S_IWUSR);
>   module_param_named(medium_error_start, sdebug_medium_error_start, int,
> @@ -7395,6 +7397,7 @@ MODULE_PARM_DESC(lbpws10, "enable LBP, support WRITE SAME(10) with UNMAP bit (de
>   MODULE_PARM_DESC(atomic_write, "enable ATOMIC WRITE support, support WRITE ATOMIC(16) (def=0)");
>   MODULE_PARM_DESC(lowest_aligned, "lowest aligned lba (def=0)");
>   MODULE_PARM_DESC(lun_format, "LUN format: 0->peripheral (def); 1 --> flat address method");
> +MODULE_PARM_DESC(max_sectors, "maximum sectors per command");
>   MODULE_PARM_DESC(max_luns, "number of LUNs per target to simulate(def=1)");
>   MODULE_PARM_DESC(max_queue, "max number of queued commands (1 to max(def))");
>   MODULE_PARM_DESC(medium_error_count, "count of sectors to return follow on MEDIUM error");
> @@ -9465,6 +9468,7 @@ static int sdebug_driver_probe(struct device *dev)
>   	hpnt->cmd_per_lun = sdebug_max_queue;
>   	if (!sdebug_clustering)
>   		hpnt->dma_boundary = PAGE_SIZE - 1;
> +	hpnt->max_sectors = sdebug_max_sectors;
>   
>   	if (submit_queues > nr_cpu_ids) {
>   		pr_warn("%s: trim submit_queues (was %d) to nr_cpu_ids=%u\n",

Regardless of my view on the merit of this patch, we still have 
max_sectors = -1 in the sht - is that still required?

Thanks,
John