Re: [PATCH v6 4/5] dm-thin: Add REQ_OP_PROVISION support

From: Mike Snitzer
Date: Fri May 12 2023 - 10:35:20 EST


On Thu, May 11 2023 at 4:03P -0400,
Sarthak Kukreti <sarthakkukreti@xxxxxxxxxxxx> wrote:

> On Tue, May 9, 2023 at 9:58 AM Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
> >
> > On Sat, May 06 2023 at 2:29P -0400,
> > Sarthak Kukreti <sarthakkukreti@xxxxxxxxxxxx> wrote:
> >
> > > 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. Additionally, if 'skip_block_zeroing'
> > > is not set, dm-thin will opt to zero out the entire range as a part
> > > of provisioning.
> > >
> > > Signed-off-by: Sarthak Kukreti <sarthakkukreti@xxxxxxxxxxxx>
> > > ---
> > > drivers/md/dm-thin.c | 70 +++++++++++++++++++++++++++++++++++++++++---
> > > 1 file changed, 66 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
> > > index 2b13c949bd72..3f94f53ac956 100644
> > > --- a/drivers/md/dm-thin.c
> > > +++ b/drivers/md/dm-thin.c
> > > @@ -4288,6 +4347,9 @@ static int thin_ctr(struct dm_target *ti, unsigned int argc, char **argv)
> > > ti->max_discard_granularity = true;
> > > }
> > >
> > > + ti->num_provision_bios = 1;
> > > + ti->provision_supported = true;
> > > +
> >
> > We need this in thin_ctr: ti->max_provision_granularity = true;
> >
> > More needed in the thin target than thin-pool; otherwise provision bio
> > issued to thin devices won't be split appropriately. But I do think
> > its fine to set in both thin_ctr and pool_ctr.
> >
> > Otherwise, looks good.
> >
> Thanks! I'll add it to the next iteration (in addition to any other
> feedback that's added to v6).

OK. I'll begin basing dm-thinp's WRITE_ZEROES support ontop of this
series.

> Given that this series covers multiple subsystems, would there be a
> preferred way of queueing this for merge?

I think it'd be OK for Jens to pick this series up and I'll rebase
my corresponding DM tree once he does.

In addition to Jens; Brian, Darrick and/or others: any chance you
could review the block core changes in this series to ensure you're
cool with them?

Would be nice to get Sarthak review feedback so that hopefully his v7
can be the final revision.

Thanks,
Mike