On 8/14/23 05:32, Damien Le Moal wrote:

quoted

On 8/12/23 06:35, Bart Van Assche wrote:

quoted

--- a/block/blk-settings.c
+++ b/block/blk-settings.c

@@ -56,6 +56,8 @@ void blk_set_default_limits(struct queue_limits *lim)
  	lim->alignment_offset = 0;
  	lim->io_opt = 0;
  	lim->misaligned = 0;
+	lim->driver_preserves_write_order = false;
+	lim->use_zone_write_lock = false;
  	lim->zoned = BLK_ZONED_NONE;
  	lim->zone_write_granularity = 0;
  	lim->dma_alignment = 511;

@@ -685,6 +687,9 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
  						   b->max_secure_erase_sectors);
  	t->zone_write_granularity = max(t->zone_write_granularity,
  					b->zone_write_granularity);
+	/* Request-based stacking drivers do not reorder requests. */
+	t->driver_preserves_write_order = b->driver_preserves_write_order;
+	t->use_zone_write_lock = b->use_zone_write_lock;

I do not think this is correct as the last target of a multi target device will
dictate the result, regardless of the other targets. So this should be something
like:

	t->driver_preserves_write_order = t->driver_preserves_write_order &&
		b->driver_preserves_write_order;
	t->use_zone_write_lock =
		t->use_zone_write_lock || b->use_zone_write_lock;

However, given that driver_preserves_write_order is initialized as false, this
would always be false. Not sure how to handle that...

How about integrating the (untested) change below into this patch? It keeps
the default value for driver_preserves_write_order to false for regular block
drivers and changes the default value to true for stacking drivers:

--- a/block/blk-settings.c
+++ b/block/blk-settings.c

@@ -84,6 +84,7 @@ void blk_set_stacking_limits(struct queue_limits *lim)
  	lim->max_dev_sectors = UINT_MAX;
  	lim->max_write_zeroes_sectors = UINT_MAX;
  	lim->max_zone_append_sectors = UINT_MAX;
+	lim->driver_preserves_write_order = true;
  }
  EXPORT_SYMBOL(blk_set_stacking_limits);

@@ -688,8 +689,10 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
  	t->zone_write_granularity = max(t->zone_write_granularity,
  					b->zone_write_granularity);
  	/* Request-based stacking drivers do not reorder requests. */
-	t->driver_preserves_write_order = b->driver_preserves_write_order;
-	t->use_zone_write_lock = b->use_zone_write_lock;
+	t->driver_preserves_write_order = t->driver_preserves_write_order &&
+		b->driver_preserves_write_order;
+	t->use_zone_write_lock = t->use_zone_write_lock ||
+		b->use_zone_write_lock;
  	t->zoned = max(t->zoned, b->zoned);
  	return ret;
  }

quoted

quoted

  	if (model != BLK_ZONED_NONE) {
  		/*
  		 * Set the zone write granularity to the device logical block

You also should set use_zone_write_lock to false in disk_clear_zone_settings().

I will do this.

quoted

In patch 9, ufshcd_auto_hibern8_update() changes the value of
driver_preserves_write_order, which will change the value of use_zone_write_lock
only if disk_set_zoned() is called again after ufshcd_auto_hibern8_update(). Is
that the case ? Is the drive revalidated always after
ufshcd_auto_hibern8_update() is executed ?

I will start with testing this change on top of this patch series:

--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c

@@ -4350,6 +4350,7 @@ static void ufshcd_update_preserves_write_order(struct ufs_hba *hba,
  		blk_mq_freeze_queue_wait(q);
  		q->limits.driver_preserves_write_order = preserves_write_order;
  		blk_mq_unfreeze_queue(q);
+		scsi_rescan_device(&sdev->sdev_gendev);