[PATCH for-3.14] dm cache: add total cache blocks to status output

[PATCH for-3.14] dm cache: add total cache blocks to status output

[Mike Snitzer]

Improve cache_status to emit <#used cache blocks>/<#total cache blocks>
This provides useful context for the how much of the cache is used.

While updating the status documentation in cache.txt spaces were
tabify'd.

Requested-by: Jonathan Brassow <jbrassow@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 Documentation/device-mapper/cache.txt |   46 +++++++++++++++++---------------
 drivers/md/dm-cache-target.c          |   10 ++++---
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/Documentation/device-mapper/cache.txt b/Documentation/device-mapper/cache.txt
index 0f41ccf..b52ac21 100644
--- a/Documentation/device-mapper/cache.txt
+++ b/Documentation/device-mapper/cache.txt
@@ -216,36 +216,38 @@ the characteristics of a specific policy, always request it by name.
 Status
 ------
 
-<#used metadata blocks>/<#total metadata blocks> <#read hits> <#read misses>
-<#write hits> <#write misses> <#demotions> <#promotions> <#blocks in cache>
-<#dirty> <#features> <features>* <#core args> <core args>* <#policy args>
-<policy args>*
-
-#used metadata blocks    : Number of metadata blocks used
-#total metadata blocks   : Total number of metadata blocks
-#read hits               : Number of times a READ bio has been mapped
+<#used metadata blocks>/<#total metadata blocks>
+<#used cache blocks>/<#total cache blocks>
+<#read hits> <#read misses> <#write hits> <#write misses>
+<#demotions> <#promotions> <#dirty> <#features> <features>*
+<#core args> <core args>* <#policy args> <policy args>*
+
+#used metadata blocks	 : Number of metadata blocks used
+#total metadata blocks	 : Total number of metadata blocks
+#used cache blocks	 : Number of blocks resident in the cache
+#total cache blocks	 : Total number of cache blocks
+#read hits		 : Number of times a READ bio has been mapped
 			     to the cache
-#read misses             : Number of times a READ bio has been mapped
+#read misses		 : Number of times a READ bio has been mapped
 			     to the origin
-#write hits              : Number of times a WRITE bio has been mapped
+#write hits		 : Number of times a WRITE bio has been mapped
 			     to the cache
-#write misses            : Number of times a WRITE bio has been
+#write misses		 : Number of times a WRITE bio has been
 			     mapped to the origin
-#demotions               : Number of times a block has been removed
+#demotions		 : Number of times a block has been removed
 			     from the cache
-#promotions              : Number of times a block has been moved to
+#promotions		 : Number of times a block has been moved to
 			     the cache
-#blocks in cache         : Number of blocks resident in the cache
-#dirty                   : Number of blocks in the cache that differ
+#dirty			 : Number of blocks in the cache that differ
 			     from the origin
-#feature args            : Number of feature args to follow
-feature args             : 'writethrough' (optional)
-#core args               : Number of core arguments (must be even)
-core args                : Key/value pairs for tuning the core
+#feature args		 : Number of feature args to follow
+feature args		 : 'writethrough' (optional)
+#core args		 : Number of core arguments (must be even)
+core args		 : Key/value pairs for tuning the core
 			     e.g. migration_threshold
-#policy args             : Number of policy arguments to follow (must be even)
-policy args              : Key/value pairs
-			     e.g. 'sequential_threshold 1024
+#policy args		 : Number of policy arguments to follow (must be even)
+policy args		 : Key/value pairs
+			     e.g. sequential_threshold
 
 Messages
 --------
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 167024b..4dee97c 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2832,8 +2832,9 @@ static void cache_resume(struct dm_target *ti)
  * Status format:
  *
  * <#used metadata blocks>/<#total metadata blocks>
+ * <#used cache blocks>/<#total cache blocks>
  * <#read hits> <#read misses> <#write hits> <#write misses>
- * <#demotions> <#promotions> <#blocks in cache> <#dirty>
+ * <#demotions> <#promotions> <#dirty>
  * <#features> <features>*
  * <#core args> <core args>
  * <#policy args> <policy args>*
@@ -2874,17 +2875,18 @@ static void cache_status(struct dm_target *ti, status_type_t type,
 
 		residency = policy_residency(cache->policy);
 
-		DMEMIT("%llu/%llu %u %u %u %u %u %u %llu %u ",
+		DMEMIT("%llu/%llu %llu/%llu %u %u %u %u %u %u %llu ",
 		       (unsigned long long)(nr_blocks_metadata - nr_free_blocks_metadata),
 		       (unsigned long long)nr_blocks_metadata,
+		       (unsigned long long) from_cblock(residency),
+		       (unsigned long long) from_cblock(cache->cache_size),
 		       (unsigned) atomic_read(&cache->stats.read_hit),
 		       (unsigned) atomic_read(&cache->stats.read_miss),
 		       (unsigned) atomic_read(&cache->stats.write_hit),
 		       (unsigned) atomic_read(&cache->stats.write_miss),
 		       (unsigned) atomic_read(&cache->stats.demotion),
 		       (unsigned) atomic_read(&cache->stats.promotion),
-		       (unsigned long long) from_cblock(residency),
-		       cache->nr_dirty);
+		       (unsigned long long) from_cblock(cache->nr_dirty));
 
 		if (writethrough_mode(&cache->features))
 			DMEMIT("1 writethrough ");

Re: [PATCH for-3.14] dm cache: add total cache blocks to status output

[Brassow Jonathan]

Yes, that'd be nice if we could have this.

It would be great if chunk_size (i.e. cache block size) was also included somehow.  If I had that, I could calculate the size of the cache using the status output alone.  I won't complain much if it isn't there though, because I can get that from the mapping table.

The reason I like adding the total number of cache blocks is because I /can't/ get that information from either type of status (_INFO or _TABLE) for the cache target.  Instead, I would have to get the cache device from the mapping table and query that device for its size - possible, but the level of indirection is a pain.  As it sits in the kernel today, it seems strange to provide some information, but not enough to fill in the whole picture.

Speaking of values I can't compute with the _INFO and _TABLE status...  "block size" does not mean the same thing for the metadata and data numbers - one is in chunk_size and the other is in something else (neither seem to be in sectors, as is DM custom).  Honestly, I'm not very sure why the ratios are provided for the metadata area... who cares?  Is it info we don't need?  No-one has ever asked if the RAID or mirror log areas are mostly full.  I don't need to worry about overfilling, do I?

 brassow

On Jan 9, 2014, at 3:04 PM, Mike Snitzer wrote:

> Improve cache_status to emit <#used cache blocks>/<#total cache blocks>
> This provides useful context for the how much of the cache is used.
> 
> While updating the status documentation in cache.txt spaces were
> tabify'd.
> 
> Requested-by: Jonathan Brassow <jbrassow@redhat.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
> Documentation/device-mapper/cache.txt |   46 +++++++++++++++++---------------
> drivers/md/dm-cache-target.c          |   10 ++++---
> 2 files changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/device-mapper/cache.txt b/Documentation/device-mapper/cache.txt
> index 0f41ccf..b52ac21 100644
> --- a/Documentation/device-mapper/cache.txt
> +++ b/Documentation/device-mapper/cache.txt
> @@ -216,36 +216,38 @@ the characteristics of a specific policy, always request it by name.
> Status
> ------
> 
> -<#used metadata blocks>/<#total metadata blocks> <#read hits> <#read misses>
> -<#write hits> <#write misses> <#demotions> <#promotions> <#blocks in cache>
> -<#dirty> <#features> <features>* <#core args> <core args>* <#policy args>
> -<policy args>*
> -
> -#used metadata blocks    : Number of metadata blocks used
> -#total metadata blocks   : Total number of metadata blocks
> -#read hits               : Number of times a READ bio has been mapped
> +<#used metadata blocks>/<#total metadata blocks>
> +<#used cache blocks>/<#total cache blocks>
> +<#read hits> <#read misses> <#write hits> <#write misses>
> +<#demotions> <#promotions> <#dirty> <#features> <features>*
> +<#core args> <core args>* <#policy args> <policy args>*
> +
> +#used metadata blocks	 : Number of metadata blocks used
> +#total metadata blocks	 : Total number of metadata blocks
> +#used cache blocks	 : Number of blocks resident in the cache
> +#total cache blocks	 : Total number of cache blocks
> +#read hits		 : Number of times a READ bio has been mapped
> 			     to the cache
> -#read misses             : Number of times a READ bio has been mapped
> +#read misses		 : Number of times a READ bio has been mapped
> 			     to the origin
> -#write hits              : Number of times a WRITE bio has been mapped
> +#write hits		 : Number of times a WRITE bio has been mapped
> 			     to the cache
> -#write misses            : Number of times a WRITE bio has been
> +#write misses		 : Number of times a WRITE bio has been
> 			     mapped to the origin
> -#demotions               : Number of times a block has been removed
> +#demotions		 : Number of times a block has been removed
> 			     from the cache
> -#promotions              : Number of times a block has been moved to
> +#promotions		 : Number of times a block has been moved to
> 			     the cache
> -#blocks in cache         : Number of blocks resident in the cache
> -#dirty                   : Number of blocks in the cache that differ
> +#dirty			 : Number of blocks in the cache that differ
> 			     from the origin
> -#feature args            : Number of feature args to follow
> -feature args             : 'writethrough' (optional)
> -#core args               : Number of core arguments (must be even)
> -core args                : Key/value pairs for tuning the core
> +#feature args		 : Number of feature args to follow
> +feature args		 : 'writethrough' (optional)
> +#core args		 : Number of core arguments (must be even)
> +core args		 : Key/value pairs for tuning the core
> 			     e.g. migration_threshold
> -#policy args             : Number of policy arguments to follow (must be even)
> -policy args              : Key/value pairs
> -			     e.g. 'sequential_threshold 1024
> +#policy args		 : Number of policy arguments to follow (must be even)
> +policy args		 : Key/value pairs
> +			     e.g. sequential_threshold
> 
> Messages
> --------
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 167024b..4dee97c 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -2832,8 +2832,9 @@ static void cache_resume(struct dm_target *ti)
>  * Status format:
>  *
>  * <#used metadata blocks>/<#total metadata blocks>
> + * <#used cache blocks>/<#total cache blocks>
>  * <#read hits> <#read misses> <#write hits> <#write misses>
> - * <#demotions> <#promotions> <#blocks in cache> <#dirty>
> + * <#demotions> <#promotions> <#dirty>
>  * <#features> <features>*
>  * <#core args> <core args>
>  * <#policy args> <policy args>*
> @@ -2874,17 +2875,18 @@ static void cache_status(struct dm_target *ti, status_type_t type,
> 
> 		residency = policy_residency(cache->policy);
> 
> -		DMEMIT("%llu/%llu %u %u %u %u %u %u %llu %u ",
> +		DMEMIT("%llu/%llu %llu/%llu %u %u %u %u %u %u %llu ",
> 		       (unsigned long long)(nr_blocks_metadata - nr_free_blocks_metadata),
> 		       (unsigned long long)nr_blocks_metadata,
> +		       (unsigned long long) from_cblock(residency),
> +		       (unsigned long long) from_cblock(cache->cache_size),
> 		       (unsigned) atomic_read(&cache->stats.read_hit),
> 		       (unsigned) atomic_read(&cache->stats.read_miss),
> 		       (unsigned) atomic_read(&cache->stats.write_hit),
> 		       (unsigned) atomic_read(&cache->stats.write_miss),
> 		       (unsigned) atomic_read(&cache->stats.demotion),
> 		       (unsigned) atomic_read(&cache->stats.promotion),
> -		       (unsigned long long) from_cblock(residency),
> -		       cache->nr_dirty);
> +		       (unsigned long long) from_cblock(cache->nr_dirty));
> 
> 		if (writethrough_mode(&cache->features))
> 			DMEMIT("1 writethrough ");

Re: [PATCH for-3.14] dm cache: add total cache blocks to status output

[Mike Snitzer]

On Thu, Jan 09 2014 at  4:28pm -0500,
Brassow Jonathan <jbrassow@redhat.com> wrote:

> Yes, that'd be nice if we could have this.
> 
> It would be great if chunk_size (i.e. cache block size) was also
> included somehow.  If I had that, I could calculate the size of the
> cache using the status output alone.  I won't complain much if it
> isn't there though, because I can get that from the mapping table.
> 
> The reason I like adding the total number of cache blocks is because I
> /can't/ get that information from either type of status (_INFO or
> _TABLE) for the cache target.  Instead, I would have to get the cache
> device from the mapping table and query that device for its size -
> possible, but the level of indirection is a pain.  As it sits in the
> kernel today, it seems strange to provide some information, but not
> enough to fill in the whole picture.

So ideally you want both the cache blocksize and the metadata blocksize.
We can easily add them, wouldn't be the end of the world.  What format
seems best?

<#used metadata blocks>/<#total metadata blocks> <metadata block size>
<#used cache blocks>/<#total cache blocks> <cache block size>
...

or:

<metadata block size> <#used metadata blocks>/<#total metadata blocks>
<cache block size> <#used cache blocks>/<#total cache blocks>
...

or something else?

(fyi, these status blocksizes would be expressed in 512b sectors)

> Speaking of values I can't compute with the _INFO and _TABLE status...
> "block size" does not mean the same thing for the metadata and data
> numbers - one is in chunk_size and the other is in something else
> (neither seem to be in sectors, as is DM custom).

The cache's block_size is in terms of 512b sectors during table load and
it is then stored in cache->sectors_per_block so I'm not sure what you
mean.

As for cache metadata block size, it is fixed at 4K (just like
dm-thin-metadata).  And yes, DM_CACHE_METADATA_BLOCK_SIZE is in bytes,
not sectors... not a big deal as we convert it to sectors when storing
it in the metadata's superblock. *shrug*

> Honestly, I'm not
> very sure why the ratios are provided for the metadata area... who
> cares?  Is it info we don't need?  No-one has ever asked if the RAID
> or mirror log areas are mostly full.  I don't need to worry about
> overfilling, do I?

If the metadata device isn't sized appropriately you can run out of
metadata space.  But in general, yes the metadata use is fixed based on
the size of the particular cache device and chosen policy (due to policy
hint size).

So we _could_ add a negative check that warns/errors to the user if the
provided metadata device isn't adequate for addressing the entire cache
device.

[PATCH v2 for-3.14] dm cache: add block sizes and total cache blocks to status output

[Mike Snitzer]

Improve cache_status to emit:
<metadata block size> <#used metadata blocks>/<#total metadata blocks>
<cache block size> <#used cache blocks>/<#total cache blocks>
...

Adding the block sizes allows for easier calculation of the overall size
of both the metadata and cache devices.  Adding <#total cache blocks>
provides useful context for how much of the cache is used.

While updating the status documentation in cache.txt spaces were
tabify'd.

Requested-by: Jonathan Brassow <jbrassow@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 Documentation/device-mapper/cache.txt |   48 ++++++++++++++++++---------------
 drivers/md/dm-cache-target.c          |   14 ++++++---
 2 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/Documentation/device-mapper/cache.txt b/Documentation/device-mapper/cache.txt
index 719320b..742c4eb 100644
--- a/Documentation/device-mapper/cache.txt
+++ b/Documentation/device-mapper/cache.txt
@@ -217,36 +217,40 @@ the characteristics of a specific policy, always request it by name.
 Status
 ------
 
-<#used metadata blocks>/<#total metadata blocks> <#read hits> <#read misses>
-<#write hits> <#write misses> <#demotions> <#promotions> <#blocks in cache>
-<#dirty> <#features> <features>* <#core args> <core args>* <#policy args>
-<policy args>*
-
-#used metadata blocks    : Number of metadata blocks used
-#total metadata blocks   : Total number of metadata blocks
-#read hits               : Number of times a READ bio has been mapped
+<metadata block size> <#used metadata blocks>/<#total metadata blocks>
+<cache block size> <#used cache blocks>/<#total cache blocks>
+<#read hits> <#read misses> <#write hits> <#write misses>
+<#demotions> <#promotions> <#dirty> <#features> <features>*
+<#core args> <core args>* <#policy args> <policy args>*
+
+metadata block size	 : Fixed block size for each metadata block
+#used metadata blocks	 : Number of metadata blocks used
+#total metadata blocks	 : Total number of metadata blocks
+cache block size	 : Configurable block size for the cache device
+#used cache blocks	 : Number of blocks resident in the cache
+#total cache blocks	 : Total number of cache blocks
+#read hits		 : Number of times a READ bio has been mapped
 			     to the cache
-#read misses             : Number of times a READ bio has been mapped
+#read misses		 : Number of times a READ bio has been mapped
 			     to the origin
-#write hits              : Number of times a WRITE bio has been mapped
+#write hits		 : Number of times a WRITE bio has been mapped
 			     to the cache
-#write misses            : Number of times a WRITE bio has been
+#write misses		 : Number of times a WRITE bio has been
 			     mapped to the origin
-#demotions               : Number of times a block has been removed
+#demotions		 : Number of times a block has been removed
 			     from the cache
-#promotions              : Number of times a block has been moved to
+#promotions		 : Number of times a block has been moved to
 			     the cache
-#blocks in cache         : Number of blocks resident in the cache
-#dirty                   : Number of blocks in the cache that differ
+#dirty			 : Number of blocks in the cache that differ
 			     from the origin
-#feature args            : Number of feature args to follow
-feature args             : 'writethrough' (optional)
-#core args               : Number of core arguments (must be even)
-core args                : Key/value pairs for tuning the core
+#feature args		 : Number of feature args to follow
+feature args		 : 'writethrough' (optional)
+#core args		 : Number of core arguments (must be even)
+core args		 : Key/value pairs for tuning the core
 			     e.g. migration_threshold
-#policy args             : Number of policy arguments to follow (must be even)
-policy args              : Key/value pairs
-			     e.g. 'sequential_threshold 1024
+#policy args		 : Number of policy arguments to follow (must be even)
+policy args		 : Key/value pairs
+			     e.g. sequential_threshold
 
 Messages
 --------
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 1b1469e..9ad7a2f 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2826,9 +2826,10 @@ static void cache_resume(struct dm_target *ti)
 /*
  * Status format:
  *
- * <#used metadata blocks>/<#total metadata blocks>
+ * <metadata block size> <#used metadata blocks>/<#total metadata blocks>
+ * <cache block size> <#used cache blocks>/<#total cache blocks>
  * <#read hits> <#read misses> <#write hits> <#write misses>
- * <#demotions> <#promotions> <#blocks in cache> <#dirty>
+ * <#demotions> <#promotions> <#dirty>
  * <#features> <features>*
  * <#core args> <core args>
  * <#policy args> <policy args>*
@@ -2869,17 +2870,20 @@ static void cache_status(struct dm_target *ti, status_type_t type,
 
 		residency = policy_residency(cache->policy);
 
-		DMEMIT("%llu/%llu %u %u %u %u %u %u %llu %u ",
+		DMEMIT("%u %llu/%llu %u %llu/%llu %u %u %u %u %u %u %llu ",
+		       (unsigned)(DM_CACHE_METADATA_BLOCK_SIZE >> SECTOR_SHIFT),
 		       (unsigned long long)(nr_blocks_metadata - nr_free_blocks_metadata),
 		       (unsigned long long)nr_blocks_metadata,
+		       cache->sectors_per_block,
+		       (unsigned long long) from_cblock(residency),
+		       (unsigned long long) from_cblock(cache->cache_size),
 		       (unsigned) atomic_read(&cache->stats.read_hit),
 		       (unsigned) atomic_read(&cache->stats.read_miss),
 		       (unsigned) atomic_read(&cache->stats.write_hit),
 		       (unsigned) atomic_read(&cache->stats.write_miss),
 		       (unsigned) atomic_read(&cache->stats.demotion),
 		       (unsigned) atomic_read(&cache->stats.promotion),
-		       (unsigned long long) from_cblock(residency),
-		       cache->nr_dirty);
+		       (unsigned long long) from_cblock(cache->nr_dirty));
 
 		if (writethrough_mode(&cache->features))
 			DMEMIT("1 writethrough ");
-- 
1.7.4.4

[PATCH] dmts: update CacheStatus to parse new status output

[Mike Snitzer]

---
 lib/dmtest/cache-status.rb |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/dmtest/cache-status.rb b/lib/dmtest/cache-status.rb
index 6c8d6c5..6fc5f96 100644
--- a/lib/dmtest/cache-status.rb
+++ b/lib/dmtest/cache-status.rb
@@ -3,8 +3,9 @@ require 'dmtest/log'
 #----------------------------------------------------------------
 
 class CacheStatus
-  attr_accessor :md_used, :md_total, :read_hits, :read_misses, :write_hits, :write_misses
-  attr_accessor :demotions, :promotions, :residency, :nr_dirty, :features, :core_args, :policy_args
+  attr_accessor :md_block_size, :md_used, :md_total, :cache_block_size, :residency, :cache_total
+  attr_accessor :read_hits, :read_misses, :write_hits, :write_misses
+  attr_accessor :demotions, :promotions, :nr_dirty, :features, :core_args, :policy_args
 
   PATTERN ='\d+\s+\d+\s+cache\s+(.*)'
 
@@ -14,14 +15,16 @@ class CacheStatus
 
     @a = m[1].split
 
+    shift_int :md_block_size
     shift_ratio :md_used, :md_total
+    shift_int :cache_block_size
+    shift_ratio :residency, :cache_total
     shift_int :read_hits
     shift_int :read_misses
     shift_int :write_hits
     shift_int :write_misses
     shift_int :demotions
     shift_int :promotions
-    shift_int :residency
     shift_int :nr_dirty
     shift_features :features
     shift_pairs :core_args

Re: [PATCH for-3.14] dm cache: add total cache blocks to status output

[Brassow Jonathan]

On Jan 9, 2014, at 4:13 PM, Mike Snitzer wrote:

> On Thu, Jan 09 2014 at  4:28pm -0500,
> Brassow Jonathan <jbrassow@redhat.com> wrote:
> 
>> Yes, that'd be nice if we could have this.
>> 
>> It would be great if chunk_size (i.e. cache block size) was also
>> included somehow.  If I had that, I could calculate the size of the
>> cache using the status output alone.  I won't complain much if it
>> isn't there though, because I can get that from the mapping table.
>> 
>> The reason I like adding the total number of cache blocks is because I
>> /can't/ get that information from either type of status (_INFO or
>> _TABLE) for the cache target.  Instead, I would have to get the cache
>> device from the mapping table and query that device for its size -
>> possible, but the level of indirection is a pain.  As it sits in the
>> kernel today, it seems strange to provide some information, but not
>> enough to fill in the whole picture.
> 
> So ideally you want both the cache blocksize and the metadata blocksize.
> We can easily add them, wouldn't be the end of the world.  What format
> seems best?
> 
> <#used metadata blocks>/<#total metadata blocks> <metadata block size>
> <#used cache blocks>/<#total cache blocks> <cache block size>
> ...
> 
> or:
> 
> <metadata block size> <#used metadata blocks>/<#total metadata blocks>
> <cache block size> <#used cache blocks>/<#total cache blocks>
> ...

Either is good (you posted the 2nd).

Still, I don't give a crap about how much metadata is used; but if you think it's useful to others, then great.  The check you mentioned to warn/error/prevent the user when providing an insufficiently sized metadata area might be nice.

Re: [PATCH for-3.14] dm cache: add total cache blocks to status output

[Joe Thornber]

On Thu, Jan 09, 2014 at 04:04:12PM -0500, Mike Snitzer wrote:
> Improve cache_status to emit <#used cache blocks>/<#total cache blocks>
> This provides useful context for the how much of the cache is used.

I'd be happy with this if cache wasn't already upstream.  But it is,
and this patch changes a kernel interface.  If you're going to do this
you need to:

i) bump the version number of the target module
ii) update the test suite
iii) apologise to users who's scripts are about to be broken.

<#used cache blocks> is already emitted.  The blocksize is available
from the table.  The <#total cache blocks> is just the SSD size
divided by the blocksize.  I see no compelling reason to make this
change.

- Joe

Re: [PATCH for-3.14] dm cache: add total cache blocks to status output

[Joe Thornber]

On Thu, Jan 09, 2014 at 03:28:13PM -0600, Brassow Jonathan wrote:
> Yes, that'd be nice if we could have this.
> 

> It would be great if chunk_size (i.e. cache block size) was also
> included somehow.  If I had that, I could calculate the size of the
> cache using the status output alone.  I won't complain much if it
> isn't there though, because I can get that from the mapping table.

Yes, either get it from the table or LVM's metadata.  This is static
information, that doesn't change during the lifetime of the cache.

> The reason I like adding the total number of cache blocks is because
> I /can't/ get that information from either type of status (_INFO or
> _TABLE) for the cache target.  Instead, I would have to get the
> cache device from the mapping table and query that device for its
> size - possible, but the level of indirection is a pain.  As it sits
> in the kernel today, it seems strange to provide some information,
> but not enough to fill in the whole picture.

You do have the information.  And since LVM set up these devices I
don't understand why it's so hard to find the size of the fast device
used in the cache.

> Speaking of values I can't compute with the _INFO and _TABLE
> status...  "block size" does not mean the same thing for the
> metadata and data numbers - one is in chunk_size and the other is in
> something else (neither seem to be in sectors, as is DM custom).
> Honestly, I'm not very sure why the ratios are provided for the
> metadata area... who cares?  Is it info we don't need?  No-one has
> ever asked if the RAID or mirror log areas are mostly full.  I don't
> need to worry about overfilling, do I?

The metadata block size is always 4k (common to dm-thin, dm-cache and
dm-era), the cache block size is variable.  For dm-thin it's
impossible to predict how much metadata space is enough, since it
depends on the number of snapshots, and the degree of metadata sharing
within those snapshots.  dm-cache is a lot more predictable, but it is
an error condition we need to check for.  For instance;

i)   The metadata device may have been created too small in the first
     place due to some bug in userland tools.

ii)  A bug in the kernel driver may cause a metadata leak (happened last month).

iii) At the moment cache policies have the option of storing a 4byte
     'hint' against each cache entry.  We do have a patch to make this
     hint size variable; this is all tested, and integrated with the
     tools.  The only reason it wasn't pushed is the policy that was
     going to use it didn't go up.  As soon as someone dreams up a
     policy that needs more space than 4 bytes then metadata use will
     become less predictable.

- Joe

Re: [PATCH for-3.14] dm cache: add total cache blocks to status output

[Joe Thornber]

On Fri, Jan 10, 2014 at 10:32:59AM +0000, Joe Thornber wrote:
> On Thu, Jan 09, 2014 at 03:28:13PM -0600, Brassow Jonathan wrote:
> > Yes, that'd be nice if we could have this.

Also, don't be fooled into thinking these changes will make your life
easier.  LVM will still have to support the current kernel interface.

- Joe

[PATCH v3 for-3.14] dm cache: add block sizes and total cache blocks to status output

[Mike Snitzer]

Improve cache_status to emit:
<metadata block size> <#used metadata blocks>/<#total metadata blocks>
<cache block size> <#used cache blocks>/<#total cache blocks>
...

Adding the block sizes allows for easier calculation of the overall size
of both the metadata and cache devices.  Adding <#total cache blocks>
provides useful context for how much of the cache is used.

Unfortunately these additions to the status will require updates to
users' scripts that monitor the cache status.  But these changes help
provide more comprehensive information about the cache device and will
simplify tools that are being developed to manage dm-cache devices --
because they won't need to issue 3 operations to cobble together the
information that we can easily provide via a single status ioctl.

While updating the status documentation in cache.txt spaces were
tabify'd.

Requested-by: Jonathan Brassow <jbrassow@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 Documentation/device-mapper/cache.txt |   50 ++++++++++++++++++--------------
 drivers/md/dm-cache-target.c          |   16 ++++++----
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/Documentation/device-mapper/cache.txt b/Documentation/device-mapper/cache.txt
index 719320b..63fd7cf 100644
--- a/Documentation/device-mapper/cache.txt
+++ b/Documentation/device-mapper/cache.txt
@@ -217,36 +217,42 @@ the characteristics of a specific policy, always request it by name.
 Status
 ------
 
-<#used metadata blocks>/<#total metadata blocks> <#read hits> <#read misses>
-<#write hits> <#write misses> <#demotions> <#promotions> <#blocks in cache>
-<#dirty> <#features> <features>* <#core args> <core args>* <#policy args>
-<policy args>*
-
-#used metadata blocks    : Number of metadata blocks used
-#total metadata blocks   : Total number of metadata blocks
-#read hits               : Number of times a READ bio has been mapped
+<metadata block size> <#used metadata blocks>/<#total metadata blocks>
+<cache block size> <#used cache blocks>/<#total cache blocks>
+<#read hits> <#read misses> <#write hits> <#write misses>
+<#demotions> <#promotions> <#dirty> <#features> <features>*
+<#core args> <core args>* <#policy args> <policy args>*
+
+metadata block size	 : Fixed block size for each metadata block in
+			     sectors
+#used metadata blocks	 : Number of metadata blocks used
+#total metadata blocks	 : Total number of metadata blocks
+cache block size	 : Configurable block size for the cache device
+			     in sectors
+#used cache blocks	 : Number of blocks resident in the cache
+#total cache blocks	 : Total number of cache blocks
+#read hits		 : Number of times a READ bio has been mapped
 			     to the cache
-#read misses             : Number of times a READ bio has been mapped
+#read misses		 : Number of times a READ bio has been mapped
 			     to the origin
-#write hits              : Number of times a WRITE bio has been mapped
+#write hits		 : Number of times a WRITE bio has been mapped
 			     to the cache
-#write misses            : Number of times a WRITE bio has been
+#write misses		 : Number of times a WRITE bio has been
 			     mapped to the origin
-#demotions               : Number of times a block has been removed
+#demotions		 : Number of times a block has been removed
 			     from the cache
-#promotions              : Number of times a block has been moved to
+#promotions		 : Number of times a block has been moved to
 			     the cache
-#blocks in cache         : Number of blocks resident in the cache
-#dirty                   : Number of blocks in the cache that differ
+#dirty			 : Number of blocks in the cache that differ
 			     from the origin
-#feature args            : Number of feature args to follow
-feature args             : 'writethrough' (optional)
-#core args               : Number of core arguments (must be even)
-core args                : Key/value pairs for tuning the core
+#feature args		 : Number of feature args to follow
+feature args		 : 'writethrough' (optional)
+#core args		 : Number of core arguments (must be even)
+core args		 : Key/value pairs for tuning the core
 			     e.g. migration_threshold
-#policy args             : Number of policy arguments to follow (must be even)
-policy args              : Key/value pairs
-			     e.g. 'sequential_threshold 1024
+#policy args		 : Number of policy arguments to follow (must be even)
+policy args		 : Key/value pairs
+			     e.g. sequential_threshold
 
 Messages
 --------
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 1b1469e..11ad705 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2826,9 +2826,10 @@ static void cache_resume(struct dm_target *ti)
 /*
  * Status format:
  *
- * <#used metadata blocks>/<#total metadata blocks>
+ * <metadata block size> <#used metadata blocks>/<#total metadata blocks>
+ * <cache block size> <#used cache blocks>/<#total cache blocks>
  * <#read hits> <#read misses> <#write hits> <#write misses>
- * <#demotions> <#promotions> <#blocks in cache> <#dirty>
+ * <#demotions> <#promotions> <#dirty>
  * <#features> <features>*
  * <#core args> <core args>
  * <#policy args> <policy args>*
@@ -2869,17 +2870,20 @@ static void cache_status(struct dm_target *ti, status_type_t type,
 
 		residency = policy_residency(cache->policy);
 
-		DMEMIT("%llu/%llu %u %u %u %u %u %u %llu %u ",
+		DMEMIT("%u %llu/%llu %u %llu/%llu %u %u %u %u %u %u %llu ",
+		       (unsigned)(DM_CACHE_METADATA_BLOCK_SIZE >> SECTOR_SHIFT),
 		       (unsigned long long)(nr_blocks_metadata - nr_free_blocks_metadata),
 		       (unsigned long long)nr_blocks_metadata,
+		       cache->sectors_per_block,
+		       (unsigned long long) from_cblock(residency),
+		       (unsigned long long) from_cblock(cache->cache_size),
 		       (unsigned) atomic_read(&cache->stats.read_hit),
 		       (unsigned) atomic_read(&cache->stats.read_miss),
 		       (unsigned) atomic_read(&cache->stats.write_hit),
 		       (unsigned) atomic_read(&cache->stats.write_miss),
 		       (unsigned) atomic_read(&cache->stats.demotion),
 		       (unsigned) atomic_read(&cache->stats.promotion),
-		       (unsigned long long) from_cblock(residency),
-		       cache->nr_dirty);
+		       (unsigned long long) from_cblock(cache->nr_dirty));
 
 		if (writethrough_mode(&cache->features))
 			DMEMIT("1 writethrough ");
@@ -3129,7 +3133,7 @@ static void cache_io_hints(struct dm_target *ti, struct queue_limits *limits)
 
 static struct target_type cache_target = {
 	.name = "cache",
-	.version = {1, 2, 0},
+	.version = {1, 3, 0},
 	.module = THIS_MODULE,
 	.ctr = cache_ctr,
 	.dtr = cache_dtr,
-- 
1.7.4.4

Re: [PATCH for-3.14] dm cache: add total cache blocks to status output

[Mike Snitzer]

On Fri, Jan 10 2014 at  5:42am -0500,
Joe Thornber <thornber@redhat.com> wrote:

> On Fri, Jan 10, 2014 at 10:32:59AM +0000, Joe Thornber wrote:
> > On Thu, Jan 09, 2014 at 03:28:13PM -0600, Brassow Jonathan wrote:
> > > Yes, that'd be nice if we could have this.
> 
> Also, don't be fooled into thinking these changes will make your life
> easier.  LVM will still have to support the current kernel interface.

LVM could easily require dm-cache >= 1.3.

Just because dm-cache was available with earlier versions doesn't mean
LVM _must_ support it.

dm-cache is EXPERIMENTAL, by definition it is subject to change.  One of
the big causes for change (as unfortunate as such changes are) is when
prominent tools like lvm2 finally get around to providing support.. when
that support is designed/implemented it can expose oversights in the
initial userspace interface provided by the kernel.  There is no hard
rule that EXPERIMENTAL dm targets must be supported by lvm2 and
conversely there is no requirement that such dm targets provide an
interface that is backwards compatible.

Re: [PATCH v3 for-3.14] dm cache: add block sizes and total cache blocks to status output

[Joe Thornber]

You're determined to do this, so I'll ack the patch.

On Fri, Jan 10, 2014 at 09:10:28AM -0500, Mike Snitzer wrote:
> Improve cache_status to emit:
> <metadata block size> <#used metadata blocks>/<#total metadata blocks>
> <cache block size> <#used cache blocks>/<#total cache blocks>
> ...
> 
> Adding the block sizes allows for easier calculation of the overall size
> of both the metadata and cache devices.  Adding <#total cache blocks>
> provides useful context for how much of the cache is used.
> 
> Unfortunately these additions to the status will require updates to
> users' scripts that monitor the cache status.  But these changes help
> provide more comprehensive information about the cache device and will
> simplify tools that are being developed to manage dm-cache devices --
> because they won't need to issue 3 operations to cobble together the
> information that we can easily provide via a single status ioctl.
> 
> While updating the status documentation in cache.txt spaces were
> tabify'd.
> 
> Requested-by: Jonathan Brassow <jbrassow@redhat.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  Documentation/device-mapper/cache.txt |   50 ++++++++++++++++++--------------
>  drivers/md/dm-cache-target.c          |   16 ++++++----
>  2 files changed, 38 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/device-mapper/cache.txt b/Documentation/device-mapper/cache.txt
> index 719320b..63fd7cf 100644
> --- a/Documentation/device-mapper/cache.txt
> +++ b/Documentation/device-mapper/cache.txt
> @@ -217,36 +217,42 @@ the characteristics of a specific policy, always request it by name.
>  Status
>  ------
>  
> -<#used metadata blocks>/<#total metadata blocks> <#read hits> <#read misses>
> -<#write hits> <#write misses> <#demotions> <#promotions> <#blocks in cache>
> -<#dirty> <#features> <features>* <#core args> <core args>* <#policy args>
> -<policy args>*
> -
> -#used metadata blocks    : Number of metadata blocks used
> -#total metadata blocks   : Total number of metadata blocks
> -#read hits               : Number of times a READ bio has been mapped
> +<metadata block size> <#used metadata blocks>/<#total metadata blocks>
> +<cache block size> <#used cache blocks>/<#total cache blocks>
> +<#read hits> <#read misses> <#write hits> <#write misses>
> +<#demotions> <#promotions> <#dirty> <#features> <features>*
> +<#core args> <core args>* <#policy args> <policy args>*
> +
> +metadata block size	 : Fixed block size for each metadata block in
> +			     sectors
> +#used metadata blocks	 : Number of metadata blocks used
> +#total metadata blocks	 : Total number of metadata blocks
> +cache block size	 : Configurable block size for the cache device
> +			     in sectors
> +#used cache blocks	 : Number of blocks resident in the cache
> +#total cache blocks	 : Total number of cache blocks
> +#read hits		 : Number of times a READ bio has been mapped
>  			     to the cache
> -#read misses             : Number of times a READ bio has been mapped
> +#read misses		 : Number of times a READ bio has been mapped
>  			     to the origin
> -#write hits              : Number of times a WRITE bio has been mapped
> +#write hits		 : Number of times a WRITE bio has been mapped
>  			     to the cache
> -#write misses            : Number of times a WRITE bio has been
> +#write misses		 : Number of times a WRITE bio has been
>  			     mapped to the origin
> -#demotions               : Number of times a block has been removed
> +#demotions		 : Number of times a block has been removed
>  			     from the cache
> -#promotions              : Number of times a block has been moved to
> +#promotions		 : Number of times a block has been moved to
>  			     the cache
> -#blocks in cache         : Number of blocks resident in the cache
> -#dirty                   : Number of blocks in the cache that differ
> +#dirty			 : Number of blocks in the cache that differ
>  			     from the origin
> -#feature args            : Number of feature args to follow
> -feature args             : 'writethrough' (optional)
> -#core args               : Number of core arguments (must be even)
> -core args                : Key/value pairs for tuning the core
> +#feature args		 : Number of feature args to follow
> +feature args		 : 'writethrough' (optional)
> +#core args		 : Number of core arguments (must be even)
> +core args		 : Key/value pairs for tuning the core
>  			     e.g. migration_threshold
> -#policy args             : Number of policy arguments to follow (must be even)
> -policy args              : Key/value pairs
> -			     e.g. 'sequential_threshold 1024
> +#policy args		 : Number of policy arguments to follow (must be even)
> +policy args		 : Key/value pairs
> +			     e.g. sequential_threshold
>  
>  Messages
>  --------
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 1b1469e..11ad705 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -2826,9 +2826,10 @@ static void cache_resume(struct dm_target *ti)
>  /*
>   * Status format:
>   *
> - * <#used metadata blocks>/<#total metadata blocks>
> + * <metadata block size> <#used metadata blocks>/<#total metadata blocks>
> + * <cache block size> <#used cache blocks>/<#total cache blocks>
>   * <#read hits> <#read misses> <#write hits> <#write misses>
> - * <#demotions> <#promotions> <#blocks in cache> <#dirty>
> + * <#demotions> <#promotions> <#dirty>
>   * <#features> <features>*
>   * <#core args> <core args>
>   * <#policy args> <policy args>*
> @@ -2869,17 +2870,20 @@ static void cache_status(struct dm_target *ti, status_type_t type,
>  
>  		residency = policy_residency(cache->policy);
>  
> -		DMEMIT("%llu/%llu %u %u %u %u %u %u %llu %u ",
> +		DMEMIT("%u %llu/%llu %u %llu/%llu %u %u %u %u %u %u %llu ",
> +		       (unsigned)(DM_CACHE_METADATA_BLOCK_SIZE >> SECTOR_SHIFT),
>  		       (unsigned long long)(nr_blocks_metadata - nr_free_blocks_metadata),
>  		       (unsigned long long)nr_blocks_metadata,
> +		       cache->sectors_per_block,
> +		       (unsigned long long) from_cblock(residency),
> +		       (unsigned long long) from_cblock(cache->cache_size),
>  		       (unsigned) atomic_read(&cache->stats.read_hit),
>  		       (unsigned) atomic_read(&cache->stats.read_miss),
>  		       (unsigned) atomic_read(&cache->stats.write_hit),
>  		       (unsigned) atomic_read(&cache->stats.write_miss),
>  		       (unsigned) atomic_read(&cache->stats.demotion),
>  		       (unsigned) atomic_read(&cache->stats.promotion),
> -		       (unsigned long long) from_cblock(residency),
> -		       cache->nr_dirty);
> +		       (unsigned long long) from_cblock(cache->nr_dirty));
>  
>  		if (writethrough_mode(&cache->features))
>  			DMEMIT("1 writethrough ");
> @@ -3129,7 +3133,7 @@ static void cache_io_hints(struct dm_target *ti, struct queue_limits *limits)
>  
>  static struct target_type cache_target = {
>  	.name = "cache",
> -	.version = {1, 2, 0},
> +	.version = {1, 3, 0},
>  	.module = THIS_MODULE,
>  	.ctr = cache_ctr,
>  	.dtr = cache_dtr,
> -- 
> 1.7.4.4
> 

Re: [PATCH v2 for-3.14] dm cache: add block sizes and total cache blocks to status output

[Brassow Jonathan]

I know I'm pushing my luck...

Is it too late to ask for just one more thing?  Would you also consider printing the policy name in the status (INFO)?
  <policy_name> <#policy args> <policy args>

This is especially useful when you pass in "default" in the CTR, because you cannot get this value from anywhere... right?

 brassow


On Jan 9, 2014, at 5:55 PM, Mike Snitzer wrote:

> Improve cache_status to emit:
> <metadata block size> <#used metadata blocks>/<#total metadata blocks>
> <cache block size> <#used cache blocks>/<#total cache blocks>
> ...
> 
> Adding the block sizes allows for easier calculation of the overall size
> of both the metadata and cache devices.  Adding <#total cache blocks>
> provides useful context for how much of the cache is used.
> 
> While updating the status documentation in cache.txt spaces were
> tabify'd.
> 
> Requested-by: Jonathan Brassow <jbrassow@redhat.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
> Documentation/device-mapper/cache.txt |   48 ++++++++++++++++++---------------
> drivers/md/dm-cache-target.c          |   14 ++++++---
> 2 files changed, 35 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/device-mapper/cache.txt b/Documentation/device-mapper/cache.txt
> index 719320b..742c4eb 100644
> --- a/Documentation/device-mapper/cache.txt
> +++ b/Documentation/device-mapper/cache.txt
> @@ -217,36 +217,40 @@ the characteristics of a specific policy, always request it by name.
> Status
> ------
> 
> -<#used metadata blocks>/<#total metadata blocks> <#read hits> <#read misses>
> -<#write hits> <#write misses> <#demotions> <#promotions> <#blocks in cache>
> -<#dirty> <#features> <features>* <#core args> <core args>* <#policy args>
> -<policy args>*
> -
> -#used metadata blocks    : Number of metadata blocks used
> -#total metadata blocks   : Total number of metadata blocks
> -#read hits               : Number of times a READ bio has been mapped
> +<metadata block size> <#used metadata blocks>/<#total metadata blocks>
> +<cache block size> <#used cache blocks>/<#total cache blocks>
> +<#read hits> <#read misses> <#write hits> <#write misses>
> +<#demotions> <#promotions> <#dirty> <#features> <features>*
> +<#core args> <core args>* <#policy args> <policy args>*
> +
> +metadata block size	 : Fixed block size for each metadata block
> +#used metadata blocks	 : Number of metadata blocks used
> +#total metadata blocks	 : Total number of metadata blocks
> +cache block size	 : Configurable block size for the cache device
> +#used cache blocks	 : Number of blocks resident in the cache
> +#total cache blocks	 : Total number of cache blocks
> +#read hits		 : Number of times a READ bio has been mapped
> 			     to the cache
> -#read misses             : Number of times a READ bio has been mapped
> +#read misses		 : Number of times a READ bio has been mapped
> 			     to the origin
> -#write hits              : Number of times a WRITE bio has been mapped
> +#write hits		 : Number of times a WRITE bio has been mapped
> 			     to the cache
> -#write misses            : Number of times a WRITE bio has been
> +#write misses		 : Number of times a WRITE bio has been
> 			     mapped to the origin
> -#demotions               : Number of times a block has been removed
> +#demotions		 : Number of times a block has been removed
> 			     from the cache
> -#promotions              : Number of times a block has been moved to
> +#promotions		 : Number of times a block has been moved to
> 			     the cache
> -#blocks in cache         : Number of blocks resident in the cache
> -#dirty                   : Number of blocks in the cache that differ
> +#dirty			 : Number of blocks in the cache that differ
> 			     from the origin
> -#feature args            : Number of feature args to follow
> -feature args             : 'writethrough' (optional)
> -#core args               : Number of core arguments (must be even)
> -core args                : Key/value pairs for tuning the core
> +#feature args		 : Number of feature args to follow
> +feature args		 : 'writethrough' (optional)
> +#core args		 : Number of core arguments (must be even)
> +core args		 : Key/value pairs for tuning the core
> 			     e.g. migration_threshold
> -#policy args             : Number of policy arguments to follow (must be even)
> -policy args              : Key/value pairs
> -			     e.g. 'sequential_threshold 1024
> +#policy args		 : Number of policy arguments to follow (must be even)
> +policy args		 : Key/value pairs
> +			     e.g. sequential_threshold
> 
> Messages
> --------
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 1b1469e..9ad7a2f 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -2826,9 +2826,10 @@ static void cache_resume(struct dm_target *ti)
> /*
>  * Status format:
>  *
> - * <#used metadata blocks>/<#total metadata blocks>
> + * <metadata block size> <#used metadata blocks>/<#total metadata blocks>
> + * <cache block size> <#used cache blocks>/<#total cache blocks>
>  * <#read hits> <#read misses> <#write hits> <#write misses>
> - * <#demotions> <#promotions> <#blocks in cache> <#dirty>
> + * <#demotions> <#promotions> <#dirty>
>  * <#features> <features>*
>  * <#core args> <core args>
>  * <#policy args> <policy args>*
> @@ -2869,17 +2870,20 @@ static void cache_status(struct dm_target *ti, status_type_t type,
> 
> 		residency = policy_residency(cache->policy);
> 
> -		DMEMIT("%llu/%llu %u %u %u %u %u %u %llu %u ",
> +		DMEMIT("%u %llu/%llu %u %llu/%llu %u %u %u %u %u %u %llu ",
> +		       (unsigned)(DM_CACHE_METADATA_BLOCK_SIZE >> SECTOR_SHIFT),
> 		       (unsigned long long)(nr_blocks_metadata - nr_free_blocks_metadata),
> 		       (unsigned long long)nr_blocks_metadata,
> +		       cache->sectors_per_block,
> +		       (unsigned long long) from_cblock(residency),
> +		       (unsigned long long) from_cblock(cache->cache_size),
> 		       (unsigned) atomic_read(&cache->stats.read_hit),
> 		       (unsigned) atomic_read(&cache->stats.read_miss),
> 		       (unsigned) atomic_read(&cache->stats.write_hit),
> 		       (unsigned) atomic_read(&cache->stats.write_miss),
> 		       (unsigned) atomic_read(&cache->stats.demotion),
> 		       (unsigned) atomic_read(&cache->stats.promotion),
> -		       (unsigned long long) from_cblock(residency),
> -		       cache->nr_dirty);
> +		       (unsigned long long) from_cblock(cache->nr_dirty));
> 
> 		if (writethrough_mode(&cache->features))
> 			DMEMIT("1 writethrough ");
> -- 
> 1.7.4.4
> 

Re: [PATCH v2 for-3.14] dm cache: add block sizes and total cache blocks to status output

[Mike Snitzer]

On Wed, Jan 15 2014 at  8:16pm -0500,
Brassow Jonathan <jbrassow@redhat.com> wrote:

> I know I'm pushing my luck...
> 
> Is it too late to ask for just one more thing?  Would you also consider printing the policy name in the status (INFO)?
>   <policy_name> <#policy args> <policy args>
> 
> This is especially useful when you pass in "default" in the CTR, because you cannot get this value from anywhere... right?

For the benefit of others, I pushed the change here:
http://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=2f20afc4c3f69030204ce55f24bc68971b500b8a

Re: [PATCH v2 for-3.14] dm cache: add block sizes and total cache blocks to status output

[Brassow Jonathan]

On Jan 15, 2014, at 10:34 PM, Mike Snitzer wrote:

> On Wed, Jan 15 2014 at  8:16pm -0500,
> Brassow Jonathan <jbrassow@redhat.com> wrote:
> 
>> I know I'm pushing my luck...
>> 
>> Is it too late to ask for just one more thing?  Would you also consider printing the policy name in the status (INFO)?
>>  <policy_name> <#policy args> <policy args>
>> 
>> This is especially useful when you pass in "default" in the CTR, because you cannot get this value from anywhere... right?
> 
> For the benefit of others, I pushed the change here:
> http://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=2f20afc4c3f69030204ce55f24bc68971b500b8a

This doesn't quite produce the info we want.  If I specify "default" in the CTR table, I want the status to show me what the result was (i.e. "mq").  It currently seems to be printing "default" back at me... which isn't helpful.  :)

 brassow

Re: [PATCH v2 for-3.14] dm cache: add block sizes and total cache blocks to status output

[Mike Snitzer]

On Thu, Jan 16 2014 at 12:18pm -0500,
Brassow Jonathan <jbrassow@redhat.com> wrote:

> 
> On Jan 15, 2014, at 10:34 PM, Mike Snitzer wrote:
> 
> > On Wed, Jan 15 2014 at  8:16pm -0500,
> > Brassow Jonathan <jbrassow@redhat.com> wrote:
> > 
> >> I know I'm pushing my luck...
> >> 
> >> Is it too late to ask for just one more thing?  Would you also consider printing the policy name in the status (INFO)?
> >>  <policy_name> <#policy args> <policy args>
> >> 
> >> This is especially useful when you pass in "default" in the CTR, because you cannot get this value from anywhere... right?
> > 
> > For the benefit of others, I pushed the change here:
> > http://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=2f20afc4c3f69030204ce55f24bc68971b500b8a
> 
> This doesn't quite produce the info we want.  If I specify "default"
> in the CTR table, I want the status to show me what the result was
> (i.e. "mq").  It currently seems to be printing "default" back at
> me... which isn't helpful.  :)

I updated the commit, see:
http://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=2e68c4e6caad9fdadc1cef8b6cb9569192e8a42b

I've also verified it works (by updating my local
device-mapper-test-suite code, will get these changes to joe soon)