Re: [PATCH 04/23] scsi: initialize scsi midlayer limits before allocating the queue

Guenter Roeck
2024-05-15T23:52:35Z

On 5/15/24 15:16, John Garry wrote:
> On 15/05/2024 15:50, Guenter Roeck wrote:
>> Hi,
>>
>> On Tue, Apr 09, 2024 at 04:37:29PM +0200, Christoph Hellwig wrote:
>>> Turn __scsi_init_queue into scsi_init_limits which initializes
>>> queue_limits structure that can be passed to blk_mq_alloc_queue.
>
> The previous behavior would sanitize max_segment_size < PAGE_SIZE, so I suppose you could try:
>
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -199,6 +199,8 @@ static int blk_validate_limits(struct queue_limits *lim)
>                  */
>                 if (!lim->max_segment_size)
>                         lim->max_segment_size = BLK_MAX_SEGMENT_SIZE;
> +               else if (lim->max_segment_size < PAGE_SIZE)
> +                       lim->max_segment_size = PAGE_SIZE;
>                 if (WARN_ON_ONCE(lim->max_segment_size < PAGE_SIZE))
>                         return -EINVAL;
>         }
>

With some debugging:

pata_macio_common_init() Calling ata_host_activate() with limit 65280
...
max_segment_size is 65280; PAGE_SIZE is 65536; BLK_MAX_SEGMENT_SIZE is 65536
WARNING: CPU: 0 PID: 12 at block/blk-settings.c:202 blk_validate_limits+0x2d4/0x364
...

This is with PPC_BOOK3S_64 which selects a default page size of 64k.
Looking at the old code, I think it did what you suggested above,

void blk_queue_max_segment_size(struct request_queue *q, unsigned int max_size)
{
if (max_size < PAGE_SIZE) {
max_size = PAGE_SIZE;
printk(KERN_INFO "%s: set to minimum %d\n",
__func__, max_size);
}
...

but assuming that the driver requested a lower limit on purpose that
may not be the best solution.

Never mind, though - I updated my test configuration to explicitly
configure the page size to 4k to work around the problem. With that,
please consider this report a note in case someone hits the problem
on a real system (and sorry for the noise).

Thanks,
Guenter