[dm-devel] [PATCH v3 2/3] dm: Add support for block provisioning

[Joe Thornber]

On Fri, Apr 14, 2023 at 7:52 AM Sarthak Kukreti <sarthakkukreti at chromium.org>
wrote:

> Add support to dm devices for REQ_OP_PROVISION. The default mode
> is to passthrough the request to the underlying device, if it
> supports it. dm-thinpool uses the provision request to provision
> blocks for a dm-thin device. dm-thinpool currently does not
> pass through REQ_OP_PROVISION to underlying devices.
>
> For shared blocks, provision requests will break sharing and copy the
> contents of the entire block.
>

I see two issue with this patch:

i) You use get_bio_block_range() to see which blocks the provision bio
covers.  But this function only returns
complete blocks that are covered (it was designed for discard).  Unlike
discard, provision is not a hint so those
partial blocks will need to be provisioned too.

ii) You are setting off multiple dm_thin_new_mapping operations in flight
at once.  Each of these receives
the same virt_cell and frees it  when it completes.  So I think we have
multiple frees occuring?  In addition you also
release the cell yourself in process_provision_cell().  Fixing this is not
trivial, you'll need to reference count the cells,
and aggregate the mapping operation results.

I think it would be far easier to restrict the size of the provision bio to
be no bigger than one thinp block (as we do for normal io).  This way dm
core can split the bios, chain the child bios rather than having to
reference count mapping ops, and aggregate the results.

- Joe
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20230414/300be4ce/attachment.htm>