Re: dm: allow a target to relax discard restrictions

[Mike Snitzer <snitzer@redhat.com>]

On Thu, Apr 28 2011 at  4:41am -0400,
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Apr 27, 2011 at 05:55:38PM -0400, Mike Snitzer wrote:
> > A target, like the upcoming thin provisioning target, may want to allow
> > discards even if the underlying devices do not support them natively.
> > 
> > Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> 
> This would work, but I thing it's really confusing to have your
> new discards_supported flag in the dm_target structure in addition
> to the existing discards_supported in the dm_table and the
> num_discard_requests field there.

It's only confusing if you allow it to be ;)  More seriously: I agree,
the target override might lend itself to confusion.

> May we should allow the target to set a tristate
> 
> enum discard_enabled {
> 	NEVER,
> 	ALWAYS,
> 	PASSTHROUGH
> }
> 
> to indicate it's discard support?

[feel free to skip to my question at the very end if I'm boring]

The above would require all targets with num_discard_requests!=0 to have
an additional explicit PASSTHROUGH opt-in.  Seems like a bit much.
Could fix that if we reordered PASSTHROUGH to be the default (0).

But I don't think having an explicit NEVER buys us much; avoids checking
the target for num_discard_requests>0 but that's not a huge win or
anything.  And it would require each target to set NEVER to get that
mini-optimization.

So that leaves:
enum discard_enabled {
       PASSTHROUGH,
       ALWAYS
}

Which is best expressed with a single flag.

With existing code (and proposed target 'discards_supported' override)
NEVER       = ti->num_discard_requests=0
ALWAYS      = ti->discards_supported=1
PASSTHROUGH = ti->num_discard_requests > 0

Would just renaming the target's 'discards_supported' override to
'discards_always_supported' help?  If not that name some other name?

Mike