[PATCH] dm-writecache: change config parameters using messages

[PATCH] dm-writecache: change config parameters using messages

[Maged Mokhtar]

Support changing configuration parameters using device-mapper messages
E.g.
    dmsetup message vg1/lv1 0 high_watermark 60

Signed-off-by: Maged Mokhtar <mmokhtar@petasan.org>
---
  drivers/md/dm-writecache.c |   68 +++++++++++++++++++++++++++++++++++
  1 file changed, 68 insertions(+)

--- a/drivers/md/dm-writecache.c	2019-08-25 16:13:54.000000000 +0200
+++ b/drivers/md/dm-writecache.c	2019-09-21 16:22:23.000000000 +0200
@@ -1009,6 +1009,69 @@ static int process_flush_on_suspend_mesg
  	return 0;
  }

+static int set_config_value(struct dm_writecache *wc, char *key, char *val)
+{
+	unsigned v,x;
+	if (sscanf(val, "%u", &v) != 1)
+		return -EINVAL;
+	if (!strcasecmp(key, "high_watermark")) {
+		if (v < 0 || v > 100)
+			return -EINVAL;
+		wc_lock(wc);
+		x = (uint64_t)wc->n_blocks * (100 - v);
+		x += 50;
+		do_div(x, 100);
+		if (wc->freelist_low_watermark < x) {
+			wc_unlock(wc);
+			return -EINVAL;
+		}
+		wc->freelist_high_watermark = x;
+		wc->high_wm_percent_set = true;
+		if (wc->freelist_size + wc->writeback_size
+			<= wc->freelist_high_watermark)
+			queue_work(wc->writeback_wq, &wc->writeback_work);
+		wc_unlock(wc);
+	}
+	else if (!strcasecmp(key, "low_watermark")) {
+		if (v < 0 || v > 100)
+			return -EINVAL;
+		wc_lock(wc);
+		x = (uint64_t)wc->n_blocks * (100 - v);
+		x += 50;
+		do_div(x, 100);
+		if (x < wc->freelist_high_watermark) {
+			wc_unlock(wc);
+			return -EINVAL;
+		}
+		wc->freelist_low_watermark = x;
+		wc->low_wm_percent_set = true;
+		wc_unlock(wc);
+	}
+	else if (!strcasecmp(key, "writeback_jobs")) {
+		wc_lock(wc);
+		wc->max_writeback_jobs = v;
+		wc->max_writeback_jobs_set = true;
+		wc_unlock(wc);
+	}
+	else if (!strcasecmp(key, "autocommit_blocks")) {
+		wc_lock(wc);
+		wc->autocommit_blocks = v;
+		wc->autocommit_blocks_set = true;
+		wc_unlock(wc);
+	}
+	else if (!strcasecmp(key, "autocommit_time")) {
+		if (v < 1 || v > 3600000)
+			return -EINVAL;
+		wc_lock(wc);
+		wc->autocommit_jiffies = msecs_to_jiffies(v);
+		wc->autocommit_time_set = true;
+		wc_unlock(wc);
+	}
+	else
+		return -EINVAL;
+	return 0;
+}
+
  static int writecache_message(struct dm_target *ti, unsigned argc, 
char **argv,
  			      char *result, unsigned maxlen)
  {
@@ -1019,6 +1082,11 @@ static int writecache_message(struct dm_
  		r = process_flush_mesg(argc, argv, wc);
  	else if (!strcasecmp(argv[0], "flush_on_suspend"))
  		r = process_flush_on_suspend_mesg(argc, argv, wc);
+	else if (argc==2) {
+		r = set_config_value(wc, argv[0], argv[1]);
+		if (r==-EINVAL)
+			DMERR("invalid message received: %s %s", argv[0], argv[1]);
+	}
  	else
  		DMERR("unrecognised message received: %s", argv[0]);

Re: [PATCH] dm-writecache: change config parameters using messages

[Maged Mokhtar]

Gentle ping please.

It could add flexibility in changing cache parameters after device creation.

On 22/09/2019 21:39, Maged Mokhtar wrote:
> Support changing configuration parameters using device-mapper messages
> E.g.
>    dmsetup message vg1/lv1 0 high_watermark 60
>
> Signed-off-by: Maged Mokhtar <mmokhtar@petasan.org>
> ---
>  drivers/md/dm-writecache.c |   68 +++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> --- a/drivers/md/dm-writecache.c    2019-08-25 16:13:54.000000000 +0200
> +++ b/drivers/md/dm-writecache.c    2019-09-21 16:22:23.000000000 +0200
> @@ -1009,6 +1009,69 @@ static int process_flush_on_suspend_mesg
>      return 0;
>  }
>
> +static int set_config_value(struct dm_writecache *wc, char *key, char 
> *val)
> +{
> +    unsigned v,x;
> +    if (sscanf(val, "%u", &v) != 1)
> +        return -EINVAL;
> +    if (!strcasecmp(key, "high_watermark")) {
> +        if (v < 0 || v > 100)
> +            return -EINVAL;
> +        wc_lock(wc);
> +        x = (uint64_t)wc->n_blocks * (100 - v);
> +        x += 50;
> +        do_div(x, 100);
> +        if (wc->freelist_low_watermark < x) {
> +            wc_unlock(wc);
> +            return -EINVAL;
> +        }
> +        wc->freelist_high_watermark = x;
> +        wc->high_wm_percent_set = true;
> +        if (wc->freelist_size + wc->writeback_size
> +            <= wc->freelist_high_watermark)
> +            queue_work(wc->writeback_wq, &wc->writeback_work);
> +        wc_unlock(wc);
> +    }
> +    else if (!strcasecmp(key, "low_watermark")) {
> +        if (v < 0 || v > 100)
> +            return -EINVAL;
> +        wc_lock(wc);
> +        x = (uint64_t)wc->n_blocks * (100 - v);
> +        x += 50;
> +        do_div(x, 100);
> +        if (x < wc->freelist_high_watermark) {
> +            wc_unlock(wc);
> +            return -EINVAL;
> +        }
> +        wc->freelist_low_watermark = x;
> +        wc->low_wm_percent_set = true;
> +        wc_unlock(wc);
> +    }
> +    else if (!strcasecmp(key, "writeback_jobs")) {
> +        wc_lock(wc);
> +        wc->max_writeback_jobs = v;
> +        wc->max_writeback_jobs_set = true;
> +        wc_unlock(wc);
> +    }
> +    else if (!strcasecmp(key, "autocommit_blocks")) {
> +        wc_lock(wc);
> +        wc->autocommit_blocks = v;
> +        wc->autocommit_blocks_set = true;
> +        wc_unlock(wc);
> +    }
> +    else if (!strcasecmp(key, "autocommit_time")) {
> +        if (v < 1 || v > 3600000)
> +            return -EINVAL;
> +        wc_lock(wc);
> +        wc->autocommit_jiffies = msecs_to_jiffies(v);
> +        wc->autocommit_time_set = true;
> +        wc_unlock(wc);
> +    }
> +    else
> +        return -EINVAL;
> +    return 0;
> +}
> +
>  static int writecache_message(struct dm_target *ti, unsigned argc, 
> char **argv,
>                    char *result, unsigned maxlen)
>  {
> @@ -1019,6 +1082,11 @@ static int writecache_message(struct dm_
>          r = process_flush_mesg(argc, argv, wc);
>      else if (!strcasecmp(argv[0], "flush_on_suspend"))
>          r = process_flush_on_suspend_mesg(argc, argv, wc);
> +    else if (argc==2) {
> +        r = set_config_value(wc, argv[0], argv[1]);
> +        if (r==-EINVAL)
> +            DMERR("invalid message received: %s %s", argv[0], argv[1]);
> +    }
>      else
>          DMERR("unrecognised message received: %s", argv[0]);
>

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

Re: dm-writecache: change config parameters using messages

[Mike Snitzer]

On Tue, Nov 05 2019 at  4:19pm -0500,
Maged Mokhtar <mmokhtar@petasan.org> wrote:

> Gentle ping please.
> 
> It could add flexibility in changing cache parameters after device creation.

I'm inclined to _not_ take this type of change.

Why isn't changing the config parameters via table reload viable for
you?

Re: dm-writecache: change config parameters using messages

[Maged Mokhtar]

On 06/11/2019 17:08, Mike Snitzer wrote:
> On Tue, Nov 05 2019 at  4:19pm -0500,
> Maged Mokhtar <mmokhtar@petasan.org> wrote:
> 
>> Gentle ping please.
>>
>> It could add flexibility in changing cache parameters after device creation.
> 
> I'm inclined to _not_ take this type of change.
> 
> Why isn't changing the config parameters via table reload viable for
> you?
> 


Hi Mike,

Thank you for your response. The main issue is to enable setting some 
config parameters while the device is mounted and running and avoid 
calling target ctr() by sending parameter changes via messages. A 
similar setup was used in dm-cache.

The reason is that tuning the write cache may require run time changes. 
If un-tuned we can observes periods of spikes and/or step like disk 
utilization on the slow device during writeback using iostat, and these 
spikes correspond to periods of increased client io latency. Utilization 
can be tuned by varying the number of active writeback jobs + the gap 
between high and low marks to achieve a smooth high utilization.  Such 
tuning is difficult to do when creating the cache device as it depends 
on the hardware and io workload. We are hoping to use some userspace 
monitoring and tuning tool to periodically adjust these values while the 
device is running.

/Maged

Re: dm-writecache: change config parameters using messages

[Mike Snitzer]

On Thu, Nov 07 2019 at  1:55pm -0500,
Maged Mokhtar <mmokhtar@petasan.org> wrote:

> 
> 
> On 06/11/2019 17:08, Mike Snitzer wrote:
> >On Tue, Nov 05 2019 at  4:19pm -0500,
> >Maged Mokhtar <mmokhtar@petasan.org> wrote:
> >
> >>Gentle ping please.
> >>
> >>It could add flexibility in changing cache parameters after device creation.
> >
> >I'm inclined to _not_ take this type of change.
> >
> >Why isn't changing the config parameters via table reload viable for
> >you?
> >
> 
> 
> Hi Mike,
> 
> Thank you for your response. The main issue is to enable setting
> some config parameters while the device is mounted and running and
> avoid calling target ctr() by sending parameter changes via
> messages. A similar setup was used in dm-cache.
> 
> The reason is that tuning the write cache may require run time
> changes. If un-tuned we can observes periods of spikes and/or step
> like disk utilization on the slow device during writeback using
> iostat, and these spikes correspond to periods of increased client
> io latency. Utilization can be tuned by varying the number of active
> writeback jobs + the gap between high and low marks to achieve a
> smooth high utilization.  Such tuning is difficult to do when
> creating the cache device as it depends on the hardware and io
> workload. We are hoping to use some userspace monitoring and tuning
> tool to periodically adjust these values while the device is
> running.

I think you're missing that any actively used DM device can be
suspended, table reloaded, resumed.  So the tuning at runtime is still
possible, it just requires more steps.

And I'm saying that unless/until there is a better reason other than
"dm-cache allowed tuning via messages" I'm not interested in having
multiple methods for tuning dm-writecache.

Mike

Re: dm-writecache: change config parameters using messages

[Maged Mokhtar]

On 07/11/2019 21:09, Mike Snitzer wrote:
> On Thu, Nov 07 2019 at  1:55pm -0500,
> Maged Mokhtar <mmokhtar@petasan.org> wrote:
> 
>>
>>
>> On 06/11/2019 17:08, Mike Snitzer wrote:
>>> On Tue, Nov 05 2019 at  4:19pm -0500,
>>> Maged Mokhtar <mmokhtar@petasan.org> wrote:
>>>
>>>> Gentle ping please.
>>>>
>>>> It could add flexibility in changing cache parameters after device creation.
>>>
>>> I'm inclined to _not_ take this type of change.
>>>
>>> Why isn't changing the config parameters via table reload viable for
>>> you?
>>>
>>
>>
>> Hi Mike,
>>
>> Thank you for your response. The main issue is to enable setting
>> some config parameters while the device is mounted and running and
>> avoid calling target ctr() by sending parameter changes via
>> messages. A similar setup was used in dm-cache.
>>
>> The reason is that tuning the write cache may require run time
>> changes. If un-tuned we can observes periods of spikes and/or step
>> like disk utilization on the slow device during writeback using
>> iostat, and these spikes correspond to periods of increased client
>> io latency. Utilization can be tuned by varying the number of active
>> writeback jobs + the gap between high and low marks to achieve a
>> smooth high utilization.  Such tuning is difficult to do when
>> creating the cache device as it depends on the hardware and io
>> workload. We are hoping to use some userspace monitoring and tuning
>> tool to periodically adjust these values while the device is
>> running.
> 
> I think you're missing that any actively used DM device can be
> suspended, table reloaded, resumed.  So the tuning at runtime is still
> possible, it just requires more steps.
> 
> And I'm saying that unless/until there is a better reason other than
> "dm-cache allowed tuning via messages" I'm not interested in having
> multiple methods for tuning dm-writecache.
> 
> Mike
> 
just for my understanding, does not table reload call _ctr() and 
re-initializes things like threads/read metadada ..?

Re: dm-writecache: change config parameters using messages

[Mike Snitzer]

On Thu, Nov 07 2019 at  2:29pm -0500,
Maged Mokhtar <mmokhtar@petasan.org> wrote:

> 
> 
> On 07/11/2019 21:09, Mike Snitzer wrote:
> >On Thu, Nov 07 2019 at  1:55pm -0500,
> >Maged Mokhtar <mmokhtar@petasan.org> wrote:
> >
> >>
> >>
> >>On 06/11/2019 17:08, Mike Snitzer wrote:
> >>>On Tue, Nov 05 2019 at  4:19pm -0500,
> >>>Maged Mokhtar <mmokhtar@petasan.org> wrote:
> >>>
> >>>>Gentle ping please.
> >>>>
> >>>>It could add flexibility in changing cache parameters after device creation.
> >>>
> >>>I'm inclined to _not_ take this type of change.
> >>>
> >>>Why isn't changing the config parameters via table reload viable for
> >>>you?
> >>>
> >>
> >>
> >>Hi Mike,
> >>
> >>Thank you for your response. The main issue is to enable setting
> >>some config parameters while the device is mounted and running and
> >>avoid calling target ctr() by sending parameter changes via
> >>messages. A similar setup was used in dm-cache.
> >>
> >>The reason is that tuning the write cache may require run time
> >>changes. If un-tuned we can observes periods of spikes and/or step
> >>like disk utilization on the slow device during writeback using
> >>iostat, and these spikes correspond to periods of increased client
> >>io latency. Utilization can be tuned by varying the number of active
> >>writeback jobs + the gap between high and low marks to achieve a
> >>smooth high utilization.  Such tuning is difficult to do when
> >>creating the cache device as it depends on the hardware and io
> >>workload. We are hoping to use some userspace monitoring and tuning
> >>tool to periodically adjust these values while the device is
> >>running.
> >
> >I think you're missing that any actively used DM device can be
> >suspended, table reloaded, resumed.  So the tuning at runtime is still
> >possible, it just requires more steps.
> >
> >And I'm saying that unless/until there is a better reason other than
> >"dm-cache allowed tuning via messages" I'm not interested in having
> >multiple methods for tuning dm-writecache.
> >
> >Mike
> >
> just for my understanding, does not table reload call _ctr() and
> re-initializes things like threads/read metadada ..?

It does, so if you can demonstrate that using suspend/reload/resume vs
your messages is cause for reduced overall performance that would help
justify your patch.

But if you're having to tune so actively that it matters I'd argue the
tuning is a workaround for a more systemic issue in the dm-writecaache
code.

Thanks,
Mike

Re: dm-writecache: change config parameters using messages

[Mikulas Patocka]

On Thu, 7 Nov 2019, Maged Mokhtar wrote:

> 
> 
> On 07/11/2019 21:09, Mike Snitzer wrote:
> > On Thu, Nov 07 2019 at  1:55pm -0500,
> > Maged Mokhtar <mmokhtar@petasan.org> wrote:
> > 
> > > 
> > > 
> > > On 06/11/2019 17:08, Mike Snitzer wrote:
> > > > On Tue, Nov 05 2019 at  4:19pm -0500,
> > > > Maged Mokhtar <mmokhtar@petasan.org> wrote:
> > > > 
> > > > > Gentle ping please.
> > > > > 
> > > > > It could add flexibility in changing cache parameters after device
> > > > > creation.
> > > > 
> > > > I'm inclined to _not_ take this type of change.
> > > > 
> > > > Why isn't changing the config parameters via table reload viable for
> > > > you?
> > > > 
> > > 
> > > 
> > > Hi Mike,
> > > 
> > > Thank you for your response. The main issue is to enable setting
> > > some config parameters while the device is mounted and running and
> > > avoid calling target ctr() by sending parameter changes via
> > > messages. A similar setup was used in dm-cache.
> > > 
> > > The reason is that tuning the write cache may require run time
> > > changes. If un-tuned we can observes periods of spikes and/or step
> > > like disk utilization on the slow device during writeback using
> > > iostat, and these spikes correspond to periods of increased client
> > > io latency. Utilization can be tuned by varying the number of active
> > > writeback jobs + the gap between high and low marks to achieve a
> > > smooth high utilization.  Such tuning is difficult to do when
> > > creating the cache device as it depends on the hardware and io
> > > workload. We are hoping to use some userspace monitoring and tuning
> > > tool to periodically adjust these values while the device is
> > > running.
> > 
> > I think you're missing that any actively used DM device can be
> > suspended, table reloaded, resumed.  So the tuning at runtime is still
> > possible, it just requires more steps.
> > 
> > And I'm saying that unless/until there is a better reason other than
> > "dm-cache allowed tuning via messages" I'm not interested in having
> > multiple methods for tuning dm-writecache.
> > 
> > Mike
> > 
> just for my understanding, does not table reload call _ctr() and
> re-initializes things like threads/read metadada ..?

Yes it does. But you don't have to unmount the filesystem when you reload 
the table.

Mikulas

Re: dm-writecache: change config parameters using messages

[Zdenek Kabelac]

Dne 07. 11. 19 v 20:50 Mikulas Patocka napsal(a):
> 
> 
> On Thu, 7 Nov 2019, Maged Mokhtar wrote:
> 
>>
>>
>> On 07/11/2019 21:09, Mike Snitzer wrote:
>>> On Thu, Nov 07 2019 at  1:55pm -0500,
>>> Maged Mokhtar <mmokhtar@petasan.org> wrote:
>>>
>>>>
>>>>
>>>> On 06/11/2019 17:08, Mike Snitzer wrote:
>>>>> On Tue, Nov 05 2019 at  4:19pm -0500,
>>>>> Maged Mokhtar <mmokhtar@petasan.org> wrote:
>>>>>
>>>>>> Gentle ping please.
>>>>>>
>>>>>> It could add flexibility in changing cache parameters after device
>>>>>> creation.
>>>>>
>>>>> I'm inclined to _not_ take this type of change.
>>>>>
>>>>> Why isn't changing the config parameters via table reload viable for
>>>>> you?
>>>>>
>>>>
>>

IMHO any target update that is changing behavior of target should be made via 
table reload - typical symptom of such requirement is - that change may
requires some memory allocation, metadata updates, synchronization of data on 
used disks...

If these things are made via messages - there is 'no simply way' back
(while with table reload - we drop table if 'suspend' cannot proceed).

Another problem is - that if change is not visible via 'table line',
future table access might be unaware of the change  and may reload
table and expect different behavior.

Also actual table update via  suspend & resume cannot be much slower then 
message anyway - you can use 'noflush' if you need to - depending on the
situation it might be usable -  and such operation is very quick.

Regards

Zdenek