Re: [PATCH v5 3/9] block: Support configuring limits below the page size

[Bart Van Assche <bvanassche@acm.org>]

On 5/26/23 20:16, Luis Chamberlain wrote:
> So I see this as a critical fix too. And it gets me wondering what has
> happened for 512 byte controllers on 4K PAGE_SIZE?

What is a "512 byte controller"? Most storage controllers I'm familiar 
with support DMA segments well above 4 KiB.

>> @@ -126,6 +173,11 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto
>>   	unsigned int min_max_hw_sectors = PAGE_SIZE >> SECTOR_SHIFT;
>>   	unsigned int max_sectors;
>>   
>> +	if (max_hw_sectors < min_max_hw_sectors) {
>> +		blk_enable_sub_page_limits(limits);
>> +		min_max_hw_sectors = 1;
>> +	}
>> +
>>   	if (max_hw_sectors < min_max_hw_sectors) {
>>   		max_hw_sectors = min_max_hw_sectors;
>>   		pr_info("%s: set to minimum %u\n", __func__, max_hw_sectors);
> 
> It would seem like max_dev_sectors would have saved the day too,
> but that is said to be set by the "disk" on the documentation.
> I see scsi/sd.c and drivers/s390/block/dasd_*.c set this too,
> is that a layering violation, or was that to help perhaps with
> similar problems? If not could stroage controller have used this
> for this issue as well?

min_not_zero(max_hw_sectors, max_dev_sectors) is the maximum transfer 
size used by the block layer. max_hw_sectors typically represents the 
transfer limit of the DMA controller and max_dev_sectors typically 
represents the transfer limit of the storage device. If e.g. a RAID 
controller exists between the host and the storage devices these limits 
can be different.

> Could the documentation for blk_queue_max_hw_sectors() be enhanced to
> clarify this?

I will look into this.

> The way I'm thinking about this is, if this is a fix for stable too,
> what would a minimum safe stable fix be like? And then after whatever
> we need to make it better (non stable fixes).

Hmm ... doesn't the "upstream first" rule apply to stable kernels? 
Shouldn't only patches land in stable kernels that are already upstream 
instead of applying different patches on stable kernels?

Thanks,

Bart.