Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Device-mapper Development (http://www.linux-archive.org/device-mapper-development/)
-   -   dm: allow a target to relax discard restrictions (http://www.linux-archive.org/device-mapper-development/519265-dm-allow-target-relax-discard-restrictions.html)

Mike Snitzer 04-27-2011 09:55 PM

dm: allow a target to relax discard restrictions
 
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>
---
drivers/md/dm-table.c | 4 ++++
include/linux/device-mapper.h | 6 ++++++
2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index cb8380c..75b0430 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1354,6 +1354,10 @@ bool dm_table_supports_discards(struct dm_table *t)
while (i < dm_table_get_num_targets(t)) {
ti = dm_table_get_target(t, i++);

+ /* target supports discards even if underlying devices cannot */
+ if (ti->discards_supported)
+ return 1;
+
if (ti->type->iterate_devices &&
ti->type->iterate_devices(ti, device_discard_capable, NULL))
return 1;
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 32a4423..c9e7ff5 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -191,6 +191,12 @@ struct dm_target {

/* Used to provide an error string from the ctr */
char *error;
+
+ /*
+ * Enable discards even if the table's underlying devices
+ * do not have native discard support.
+ */
+ unsigned discards_supported:1;
};

/* Each target can link one of these into the table */
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Christoph Hellwig 04-28-2011 08:41 AM

dm: allow a target to relax discard restrictions
 
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.

May we should allow the target to set a tristate

enum discard_enabled {
NEVER,
ALWAYS,
PASSTHROUGH
}

to indicate it's discard support?

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Mike Snitzer 04-28-2011 03:04 PM

dm: allow a target to relax discard restrictions
 
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
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


All times are GMT. The time now is 11:01 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.