[dm-devel] Why queue_work unneeded for REQUEUE bio

[JeffleXu]

On 11/6/20 11:21 PM, Mike Snitzer wrote:
> On Thu, Nov 05 2020 at 10:49pm -0500,
> JeffleXu <jefflexu at linux.alibaba.com> wrote:
>
>> Hi Mike,
>>
>>
>> I have another question about dm, though it's irrelevant to this original
>>
>> mail.
>>
>>
>> Currently abnormal IO will call blk_queue_split() and normal IO will
>>
>> be split considering @max_sectors/@chunk_sectos (in max_io_len()).
>>
>>
>> Question 1: Why bio should be split considering queue_limits in dm layer?
>>
>> After all the underlying device will split themselves by queue_limits if
>>
>> the dm layer doesn't split by queue_limits.
> Some targets have "abnormal IO" constraints in their implementation that
> is reflected in the queue_limits -- discards in particular.
>
>> Then Question 2: Currently only @max_sectors is considered when splitting
>>
>> normal IO in dm layer. Should we also consider
>> @max_segments/@max_segment_size
>>
>> as blk_queue_split() does?
> Great question, it does appear the one gap in DM's splitting for
> "normal" IO. I'm less familiar with @max_segments/@max_segment_size.
>
> Since commit 5091cdec56fa ("dm: change max_io_len() to use
> blk_max_size_offset()") DM is making use of more block core code to
> calculate its splits -- the offset based splitting is much more natural
> for DM to perform given that potential for spanning multiple targets,
> etc.  But DM targets really don't get involved with concern for
> @max_segments/@max_segment_size
>
> dm-crypt.c:crypt_io_hints is the only DM target code that concerns
> itself with @max_segment_size -- and it is punitive by setting it to
> PAGE_SIZE, please see commit 586b286b110e94e ("dm crypt: constrain crypt
> device's max_segment_size to PAGE_SIZE") for more context.

Thanks. So the principle of handling queue_limits of dm layer is that,  
only when dm-target

specific queue_limits is set, shall we consider queue_limitswhen 
splitting bio in dm, in

which case these queue_limits (from underlying devices) are left for the 
underlying devices

to be handled appropriately.


(In this case there's no dm-target set its own 
@max_segments/@max_segment_size, then

these two parameters are not considered when dm splitting.)


If that's the case, then why bother stacking queue_limits in 
blk_stack_limits()? Such as

```

blk_stack_limits

     t->max_segments = min_not_zero(t->max_segments, b->max_segments);
     t->max_segment_size = min_not_zero(t->max_segment_size,
                        b->max_segment_size);

```


I'm not challenging the implementation. Of course it works fine 
currently. I'm just curious

about the design consideration beneath the implementation. :-)


-- 
Thanks,
Jeffle