On 2/1/23 15:50, Luis Chamberlain wrote:

On Mon, Jan 30, 2023 at 01:26:51PM -0800, Bart Van Assche wrote:

quoted

@@ -122,12 +177,17 @@ EXPORT_SYMBOL(blk_queue_bounce_limit);
  void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_sectors)
  {
  	struct queue_limits *limits = &q->limits;
+	unsigned int min_max_hw_sectors = PAGE_SIZE >> SECTOR_SHIFT;
  	unsigned int max_sectors;
  
-	if ((max_hw_sectors << 9) < PAGE_SIZE) {
-		max_hw_sectors = 1 << (PAGE_SHIFT - 9);
-		printk(KERN_INFO "%s: set to minimum %d\n",
-		       __func__, max_hw_sectors);
+	if (max_hw_sectors < min_max_hw_sectors) {
+		blk_enable_sub_page_limits(limits);
+		min_max_hw_sectors = 1;
+	}

Up to this part this a non-functional update, and so why not a
separate patch to clarify that.

Will do.

quoted

+
+	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);

But if I understand correctly here we're now changing
max_hw_sectors from 1 to whatever the driver set on
blk_queue_max_hw_sectors() if its smaller than PAGE_SIZE.

To determine if this is a functional change it begs the
question as to how many block drivers have a max hw sector
smaller than the equivalent PAGE_SIZE and wonder if that
could regress.

If a block driver passes an argument to blk_queue_max_hw_sectors() or 
blk_queue_max_segment_size() that is smaller than what is supported by 
the block layer, data corruption will be triggered if the block driver 
or the hardware supported by the block driver does not support the 
larger values chosen by the block layer. Changing limits that will 
trigger data corruption into limits that may work seems like an 
improvement to me?

Thanks,

Bart.