Re: [PATCH 4/4] block: simplify tag allocation policy selection

John Garry
2025-01-06T08:50:36Z

On 06/01/2025 08:35, Christoph Hellwig wrote:
> Use a plain BLK_MQ_F_* flag to select the round robin tag selection
> instead of overlaying an enum with just two possible values into the
> flags space.
>
> Doing so allows adding a BLK_MQ_F_MAX sentinel for simplified overflow
> checking in the messy debugfs helpers.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Looks fine, but some small comments still. However, feel free to add:

Reviewed-by: John Garry <john.g.garry@oracle.com>

> /*
> diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
> index 72c03cbdaff4..935b13e79dec 100644
> --- a/drivers/ata/sata_sil24.c
> +++ b/drivers/ata/sata_sil24.c
> @@ -378,7 +378,6 @@ static const struct scsi_host_template sil24_sht = {
> .can_queue = SIL24_MAX_CMDS,
> .sg_tablesize = SIL24_MAX_SGE,
> .dma_boundary = ATA_DMA_BOUNDARY,
> - .tag_alloc_policy = BLK_TAG_ALLOC_FIFO,

nit: maybe that could be a separate patch, but no biggie

> .sdev_groups = ata_ncq_sdev_groups,
> .change_queue_depth = ata_scsi_change_queue_depth,
> .device_configure = ata_scsi_device_configure
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index 79129c977704..35501d0aa655 100644

> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -438,8 +438,10 @@ struct scsi_host_template {
> */
> short cmd_per_lun;
>
> - /* If use block layer to manage tags, this is tag allocation policy */
> - int tag_alloc_policy;
> + /*
> + * Allocate tags starting from last allocated tag.
> + */
> + bool tag_alloc_policy_rr : 1;

Is it proper to use bool here? I am not sure. Others use unsigned int or
unsigned.

nit: coding style elsewhere in scsi_host_template would be to use
"tag_alloc_policy_rr:1;"

>
> /*
> * Track QUEUE_FULL events and reduce queue depth on demand.