Re: [PATCH v15 01/19] block: Introduce more member variables related to zone write locking

Bart Van Assche
2023-11-20T20:44:48Z

On 11/19/23 15:29, Damien Le Moal wrote:
> On 11/15/23 06:16, Bart Van Assche wrote:
>> @@ -82,6 +84,8 @@ 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;
>> + /* Request-based stacking drivers do not reorder requests. */
>
> Rereading this patch, I do not think this statement is correct. I seriously
> doubt that multipath will preserve write command order in all cases...
>
>> + lim->driver_preserves_write_order = true;
>
> ... so it is likely much safer to set the default to "false" as that is the
> default for all requests in general.

How about applying this (untested) patch on top of this patch series?

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 4c776c08f190..aba1972e9767 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -84,8 +84,6 @@ 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;
- /* Request-based stacking drivers do not reorder requests. */
- lim->driver_preserves_write_order = true;
}
EXPORT_SYMBOL(blk_set_stacking_limits);

diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 2d3e186ca87e..cb9abe4bd065 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -147,6 +147,11 @@ static int linear_report_zones(struct dm_target *ti,
#define linear_report_zones NULL
#endif

+static void linear_io_hints(struct dm_target *ti, struct queue_limits *limits)
+{
+ limits->driver_preserves_write_order = true;
+}
+
static int linear_iterate_devices(struct dm_target *ti,
iterate_devices_callout_fn fn, void *data)
{
@@ -208,6 +213,7 @@ static struct target_type linear_target = {
.map = linear_map,
.status = linear_status,
.prepare_ioctl = linear_prepare_ioctl,
+ .io_hints = linear_io_hints,
.iterate_devices = linear_iterate_devices,
.direct_access = linear_dax_direct_access,
.dax_zero_page_range = linear_dax_zero_page_range,

>> @@ -685,6 +689,10 @@ 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);
>> + 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;
>
> Very minor nit: splitting the line after the equal would make this more readable.

Hmm ... I have often seen other reviewers asking to maximize the use of each
source code line as much as reasonably possible.

Thanks,

Bart.