* [PATCH] dm-writecache: change config parameters using messages
@ 2019-09-22 19:39 Maged Mokhtar
2019-11-05 21:19 ` Maged Mokhtar
0 siblings, 1 reply; 9+ messages in thread
From: Maged Mokhtar @ 2019-09-22 19:39 UTC (permalink / raw)
To: mpatocka; +Cc: dm-devel
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]);
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] dm-writecache: change config parameters using messages
2019-09-22 19:39 [PATCH] dm-writecache: change config parameters using messages Maged Mokhtar
@ 2019-11-05 21:19 ` Maged Mokhtar
2019-11-06 15:08 ` Mike Snitzer
0 siblings, 1 reply; 9+ messages in thread
From: Maged Mokhtar @ 2019-11-05 21:19 UTC (permalink / raw)
To: mpatocka; +Cc: dm-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: dm-writecache: change config parameters using messages
2019-11-05 21:19 ` Maged Mokhtar
@ 2019-11-06 15:08 ` Mike Snitzer
2019-11-07 18:55 ` Maged Mokhtar
0 siblings, 1 reply; 9+ messages in thread
From: Mike Snitzer @ 2019-11-06 15:08 UTC (permalink / raw)
To: Maged Mokhtar; +Cc: dm-devel, mpatocka
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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: dm-writecache: change config parameters using messages
2019-11-06 15:08 ` Mike Snitzer
@ 2019-11-07 18:55 ` Maged Mokhtar
2019-11-07 19:09 ` Mike Snitzer
0 siblings, 1 reply; 9+ messages in thread
From: Maged Mokhtar @ 2019-11-07 18:55 UTC (permalink / raw)
To: Mike Snitzer; +Cc: dm-devel, mpatocka
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: dm-writecache: change config parameters using messages
2019-11-07 18:55 ` Maged Mokhtar
@ 2019-11-07 19:09 ` Mike Snitzer
2019-11-07 19:29 ` Maged Mokhtar
0 siblings, 1 reply; 9+ messages in thread
From: Mike Snitzer @ 2019-11-07 19:09 UTC (permalink / raw)
To: Maged Mokhtar; +Cc: dm-devel, mpatocka
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: dm-writecache: change config parameters using messages
2019-11-07 19:09 ` Mike Snitzer
@ 2019-11-07 19:29 ` Maged Mokhtar
2019-11-07 19:49 ` Mike Snitzer
2019-11-07 19:50 ` Mikulas Patocka
0 siblings, 2 replies; 9+ messages in thread
From: Maged Mokhtar @ 2019-11-07 19:29 UTC (permalink / raw)
To: Mike Snitzer; +Cc: dm-devel, mpatocka
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 ..?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: dm-writecache: change config parameters using messages
2019-11-07 19:29 ` Maged Mokhtar
@ 2019-11-07 19:49 ` Mike Snitzer
2019-11-07 19:50 ` Mikulas Patocka
1 sibling, 0 replies; 9+ messages in thread
From: Mike Snitzer @ 2019-11-07 19:49 UTC (permalink / raw)
To: Maged Mokhtar; +Cc: dm-devel, mpatocka
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: dm-writecache: change config parameters using messages
2019-11-07 19:29 ` Maged Mokhtar
2019-11-07 19:49 ` Mike Snitzer
@ 2019-11-07 19:50 ` Mikulas Patocka
2019-11-07 20:16 ` Zdenek Kabelac
1 sibling, 1 reply; 9+ messages in thread
From: Mikulas Patocka @ 2019-11-07 19:50 UTC (permalink / raw)
To: Maged Mokhtar; +Cc: dm-devel, Mike Snitzer
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: dm-writecache: change config parameters using messages
2019-11-07 19:50 ` Mikulas Patocka
@ 2019-11-07 20:16 ` Zdenek Kabelac
0 siblings, 0 replies; 9+ messages in thread
From: Zdenek Kabelac @ 2019-11-07 20:16 UTC (permalink / raw)
To: Mikulas Patocka, Maged Mokhtar; +Cc: dm-devel, Mike Snitzer
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-07 20:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-22 19:39 [PATCH] dm-writecache: change config parameters using messages Maged Mokhtar
2019-11-05 21:19 ` Maged Mokhtar
2019-11-06 15:08 ` Mike Snitzer
2019-11-07 18:55 ` Maged Mokhtar
2019-11-07 19:09 ` Mike Snitzer
2019-11-07 19:29 ` Maged Mokhtar
2019-11-07 19:49 ` Mike Snitzer
2019-11-07 19:50 ` Mikulas Patocka
2019-11-07 20:16 ` Zdenek Kabelac
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.