[Question] dm-cache table

[Question] dm-cache table

[Akira Hayakawa]

Hi,

dm-cache saves the ctr args in .ctr (copy_ctr_copy)
and uses it to make table. 

        case STATUSTYPE_TABLE:
                format_dev_t(buf, cache->metadata_dev->bdev->bd_dev);
                DMEMIT("%s ", buf);
                format_dev_t(buf, cache->cache_dev->bdev->bd_dev);
                DMEMIT("%s ", buf);
                format_dev_t(buf, cache->origin_dev->bdev->bd_dev);
                DMEMIT("%s", buf);

                for (i = 0; i < cache->nr_ctr_args - 1; i++)
                        DMEMIT(" %s", cache->ctr_args[i]);
                if (cache->nr_ctr_args)
                        DMEMIT(" %s", cache->ctr_args[cache->nr_ctr_args - 1]);
        }

If it accepted migrate_threshold in .ctr and the parameter
changed later. The actual value and what is seen in table
become inconsistent right? Is this intentionally designed?

If table is just the copy of the ctr args,
why don't we implement it in dm framework?

Akira

Re: [Question] dm-cache table

[Joe Thornber]

On Mon, Nov 25, 2013 at 07:54:39PM +0900, Akira Hayakawa wrote:
> If it accepted migrate_threshold in .ctr and the parameter
> changed later. The actual value and what is seen in table
> become inconsistent right? Is this intentionally designed?

No, this is a bug.

> If table is just the copy of the ctr args,
> why don't we implement it in dm framework?

It has been discussed.  Originally the target were so simple it was
easy to generate a new table line.  Creating copies of the table
params only happened recently with the more complicated targets.  We
could have some infrastructure provided by the dm core for this, but
we can't use it blindly as your bug above demonstrates.

- Joe

Re: [Question] dm-cache table

[Akira Hayakawa]

Joe,

> On Mon, Nov 25, 2013 at 07:54:39PM +0900, Akira Hayakawa wrote:
>> If it accepted migrate_threshold in .ctr and the parameter
>> changed later. The actual value and what is seen in table
>> become inconsistent right? Is this intentionally designed?
> 
> No, this is a bug.

So, the table design should be fully consistent with the actual values?

The only essential property of the table output is that it can
recreate the same device?
Your device-mapper-test-suite seems to use this property to make
a thoroughly new device to test.

I am wondering the table design in writeboost.
writeboost takes 'optional args' in .ctr each defined default value.
Let the values name k1 and k2, and the default values v1 and v2.
Assume only k1 v1' (!= v1) is passed to the .ctr in building the device
and the internal values are {k1:v1', k2:v2} of course then
what should the table look like? k2 v2 should included?

Also, writeboost takes 'tunable args' following the optional args.
They can be changed while running.
Should the table be consistent with the updated values is my wondering
that makes me ask you the question in the previous mail.
There could be an another way of thinking about the tunable things
that it doesn't have to be included in table because it can tuned
after all.


Just for information, this is the current sample lines
to initialize writeboost device.
dmsetup create writeboost-vol --table "0 ${sz} 0 writeboost ${BACKING} {CACHE}"
dmsetup create writeboost-vol --table "0 ${sz} 0 writeboost ${BACKING} {CACHE} \
                                       4 rambuf_pool_amount 8192 segment_size_order 8 \
                                       2 allow_migrate 1"
dmsetup create writeboost-vol --table "0 ${sz} 0 writeboost ${BACKING} {CACHE} \
                                       0 \
                                       2 allow_migrate 1"


Thanks,
Akira

Re: [Question] dm-cache table

[Alasdair G Kergon]

On Wed, Nov 27, 2013 at 10:54:04AM +0900, Akira Hayakawa wrote:
> The only essential property of the table output is that it can
> recreate the same device?

There is a further constraint.

Userspace often uses the optimisation of comparing the table line output
from the existing device with a proposed new table line to determine
whether or not the table has changed and needs to be reloaded.  For
simplicity, this can be a string comparison.

As such, the output table line that corresponds to any particular
internal table state should always be defined uniquely, particularly if
there is more than input table line that leads to that same table state.
Typically this means one of two things: EITHER arbitrary choices (e.g.
the order of supplied arguments in an unordered list; device references)
are preserved between the input and output table lines OR (normally
the better option) the output is always presented in the same standard
format regardless of the input (e.g. unordered parameter lists are
output in a defined order).  This means that userspace code that is
aware of the table output definition can choose to provide input in an
exactly matching form, but the casual user is not inhibited by such 
arbitrary restrictions.

Alasdair

Re: [Question] dm-cache table

[Joe Thornber]

On Wed, Nov 27, 2013 at 10:54:04AM +0900, Akira Hayakawa wrote:
> Joe,
> 
> > On Mon, Nov 25, 2013 at 07:54:39PM +0900, Akira Hayakawa wrote:
> >> If it accepted migrate_threshold in .ctr and the parameter
> >> changed later. The actual value and what is seen in table
> >> become inconsistent right? Is this intentionally designed?
> > 
> > No, this is a bug.
> 
> So, the table design should be fully consistent with the actual values?

I guess the status should show what's running, and the table line
should show what was originally loaded.  The problem we have here is
that original table can go out of date and mislead any volume managers
that are relying on it; I really would like to keep the following
sequence a logical noop for all targets:

  - load table
  - run for a bit
  - read table via STATUS ioctl call
  - load table just read
  - switch to newly loaded table

Putting those tunables on the target line and then having messages to
change them obviously breaks this (though not in a data loss way).

The other option would be to store the tunables in the metadata, and
have them only changed via messages (not on the target line).  The
draw back with this approach is it puts extra work on the userland
volume management software; either they end up duplicating these
settings in their own metadata, or have to be able to query them from
the metadata (remember the volume may not be active).  I suggest you
go with this approach for now.

- Joe

Re: [Question] dm-cache table

[Akira Hayakawa]

First of all, this is the current Writeboost's table line design:
writeboost <type> <backing dev> <cache dev>
           #optional args [optional args]
	   #tunable args [tunable args] 

There's a constraint.
Joe's device-mapper-test-suite uses table line output to clone a device to test.
This means getting rid of the tunables from the line will make it incapable of
testing _tuned_ device.

As Alasdair said,
there are userland tools that compares the representations of output and what to be input
to avoid unnecessary reload. My first thought is to remove these worthless
optimization.
Other option is to define .table_eq :: String -> Bool method in target and
allow users to use it for that checking where the default behavior is left
as string comparison.

Alasdair's suggestion that the output design is always unique so that
the user can make their input as in matching form and string comparison works.
However, this option will discard the benefit of leaving those arguments
optional because it always need to input full set of arguments.
But, this looks the most sane because the user land tool anyway creates
the full set of arguments (They don't use the default value set
by the target). The optional property of some part of the table line
is only beneficial for those who directly uses dmsetup command.

The drawback of requiring full set of arguments is that
it's hard to add arguments anymore after released for backward-compatibility
but we can deal with this problem by remembering what arguments are
originally input (and the ordering) and includes those in output.

> I guess the status should show what's running, and the table line
> should show what was originally loaded.
My idea can go along with your idea by not including the tunables in
table line output if what was originally loaded included none.

> The other option would be to store the tunables in the metadata, and
> have them only changed via messages (not on the target line).  The
> draw back with this approach is it puts extra work on the userland
> volume management software; either they end up duplicating these
> settings in their own metadata, or have to be able to query them from
> the metadata (remember the volume may not be active).  I suggest you
> go with this approach for now.
This is not about the table design.
Remembering tunables in cache metadata and applying them at resuming
under .ctr sounds nice but userland takes responsible for this setup things
is my opinion.


Akira