On Fri, Apr 14, 2023 at 7:52 AM Sarthak Kukreti <sarthakkukreti@xxxxxxxxxxxx> 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
-- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
