On Thu, Dec 03, 2020 at 09:03:43PM -0500, Mike Snitzer wrote:

On Thu, Dec 03 2020 at  8:12pm -0500,
Ming Lei [off-list ref] wrote:

quoted

On Thu, Dec 03, 2020 at 09:33:59AM -0500, Mike Snitzer wrote:

quoted

On Wed, Dec 02 2020 at 10:26pm -0500,
Ming Lei [off-list ref] wrote:

quoted

On Tue, Dec 01, 2020 at 11:07:09AM -0500, Mike Snitzer wrote:

quoted

commit 22ada802ede8 ("block: use lcm_not_zero() when stacking
chunk_sectors") broke chunk_sectors limit stacking. chunk_sectors must
reflect the most limited of all devices in the IO stack.

Otherwise malformed IO may result. E.g.: prior to this fix,
->chunk_sectors = lcm_not_zero(8, 128) would result in
blk_max_size_offset() splitting IO at 128 sectors rather than the
required more restrictive 8 sectors.

What is the user-visible result of splitting IO at 128 sectors?

The VDO dm target fails because it requires IO it receives to be split
as it advertised (8 sectors).

OK, looks VDO's chunk_sector limit is one hard constraint, even though it
is one DM device, so I guess you are talking about DM over VDO?

Another reason should be that VDO doesn't use blk_queue_split(), otherwise it
won't be a trouble, right?

Frankly speaking, if the stacking driver/device has its own hard queue limit
like normal hardware drive, the driver should be responsible for the splitting.

DM core does the splitting for VDO (just like any other DM target).
In 5.9 I updated DM to use chunk_sectors, use blk_stack_limits()
stacking of it, and also use blk_max_size_offset().

But all that block core code has shown itself to be too rigid for DM.  I
tried to force the issue by stacking DM targets' ti->max_io_len with
chunk_sectors.  But really I'd need to be able to pass in the per-target
max_io_len to blk_max_size_offset() to salvage using it.

Stacking chunk_sectors seems ill-conceived.  One size-fits-all splitting
is too rigid.

DM/VDO knows exactly it is one hard chunk_sectors limit, and DM shouldn't play
the stacking trick on VDO's chunk_sectors limit, should it?


Thanks, 
Ming