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