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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/21/23 05:44, Bart Van Assche wrote:
> 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;
> +}

Hmm, but does dm-linear preserve write order ? I am not convinced. And what
about dm-flakey, dm-error and dm-crypt ? All of these also support zoned
devices. I do not think that we can say that any of these preserve write order.

> +
>   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.

As I said, very minor nit :) Feel free to ignore.

> 
> Thanks,
> 
> Bart.
> 

-- 
Damien Le Moal
Western Digital Research





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux