timer: provide an api for deferrable timeout

KSM thread to scan pages is scheduled on definite timeout. That wakes up CPU from idle state and hence may affect the power consumption. Provide an optional support to use deferrable timer which suites low-power use-cases.

Typically, on our setup we observed, 10% less power consumption with some use-cases in which CPU goes to power collapse frequently. For example, playing audio on Soc which has HW based Audio encoder/decoder, CPU remains idle for longer duration of time. This idle state will save significant CPU power consumption if KSM don't wakes them up periodically.

Note that, deferrable timers won't be deferred if any CPU is active and not in IDLE state.

By default, deferrable timers is enabled. To disable deferrable timers,

$ echo 0 > /sys/kernel/mm/ksm/deferrable_timer

Signed-off-by Chintan Pandya <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc John Stultz <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Hugh Dickins <[email protected]>

---
Changes:

V3-->V4:
- Use deferrable timers by default

V2-->V3:
- Handled error case properly
- Corrected indentation in Documentation
- Fixed build failure
- Removed left over process_timeout()
V1-->V2:
- allowing only valid values to be updated as use_deferrable_timer
- using only 'deferrable' and not 'deferred'
- moved out schedule_timeout code for deferrable timer into timer.c

 Documentation/vm/ksm.txt |  6 ++++++
 mm/ksm.c                 | 36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

Documentation/vm/ksm.txt +0 -0

index f34a8ee..23e26c3 100644

mm/ksm.c +8 -2

+
 #ifdef CONFIG_NUMA
 /* Zeroed when merging across nodes is not allowed */
 static unsigned int ksm_merge_across_nodes = 1;

@@ -1725,8 +1728,13 @@ static int ksm_scan_thread(void *nothing)
 		try_to_freeze();
 
 		if (ksmd_should_run()) {
-			schedule_timeout_interruptible(
-				msecs_to_jiffies(ksm_thread_sleep_millisecs));
+			signed long to;
+
+			to = msecs_to_jiffies(ksm_thread_sleep_millisecs);
+			if (use_deferrable_timer)
+				schedule_timeout_deferrable_interruptible(to);
+			else
+				schedule_timeout_interruptible(to);
 		} else {
 			wait_event_freezable(ksm_thread_wait,
 				ksmd_should_run() || kthread_should_stop());

mm/ksm.c +7 -0

+		return -EINVAL;
+	use_deferrable_timer = enable;
+	return count;
+}
+KSM_ATTR(deferrable_timer);
+
 #ifdef CONFIG_NUMA
 static ssize_t merge_across_nodes_show(struct kobject *kobj,
 				struct kobj_attribute *attr, char *buf)

@@ -2287,6 +2318,7 @@ static struct attribute *ksm_attrs[] = {
 	&pages_unshared_attr.attr,
 	&pages_volatile_attr.attr,
 	&full_scans_attr.attr,
+	&deferrable_timer_attr.attr,
 #ifdef CONFIG_NUMA
 	&merge_across_nodes_attr.attr,
 #endif

Signature · 4 lines

Chintan Pandya

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

Sorry for holding you up, I'm slow. and needed to think about this more,

Quoted 12 lines · Chintan Pandya

KSM thread to scan pages is scheduled on definite timeout. That wakes up CPU from idle state and hence may affect the power consumption. Provide an optional support to use deferrable timer which suites low-power use-cases.

Typically, on our setup we observed, 10% less power consumption with some use-cases in which CPU goes to power collapse frequently. For example, playing audio on Soc which has HW based Audio encoder/decoder, CPU remains idle for longer duration of time. This idle state will save significant CPU power consumption if KSM don't wakes them up periodically.

Quoted 5 lines

Note that, deferrable timers won't be deferred if any CPU is active and not in IDLE state.

By default, deferrable timers is enabled. To disable deferrable timers,

$ echo 0 > /sys/kernel/mm/ksm/deferrable_timer

I have now experimented. And, much as I wanted to eliminate the tunable, and just have deferrable timers on, I have come right back to your original position.

I was impressed by how quiet ksmd goes when there's nothing much happening on the machine; but equally, disappointed in how slow it then is to fulfil the outstanding merge work. I agree with your original assessment, that not everybody will want deferrable timer, the way it is working at present.

I expect that can be fixed, partly by doing more work on wakeup from a deferred timer, according to how long it has been deferred; and partly by not deferring on idle until two passes of the list have been completed. But that's easier said than done, and might turn out to

Quoted 16 lines · Hugh Dickins

Sorry for holding you up, I'm slow. and needed to think about this more,

Quoted 12 lines · Chintan Pandya

KSM thread to scan pages is scheduled on definite timeout. That wakes up CPU from idle state and hence may affect the power consumption. Provide an optional support to use deferrable timer which suites low-power use-cases.

Typically, on our setup we observed, 10% less power consumption with some use-cases in which CPU goes to power collapse frequently. For example, playing audio on Soc which has HW based Audio encoder/decoder, CPU remains idle for longer duration of time. This idle state will save significant CPU power consumption if KSM don't wakes them up periodically.

Quoted 2 lines

Quoted 2 lines

Note that, deferrable timers won't be deferred if any CPU is active and not in IDLE state.

Quoted 13 lines

Quoted 2 lines

By default, deferrable timers is enabled. To disable deferrable timers,

$ echo 0 > /sys/kernel/mm/ksm/deferrable_timer

I have now experimented. And, much as I wanted to eliminate the tunable, and just have deferrable timers on, I have come right back to your original position.

I was impressed by how quiet ksmd goes when there's nothing much happening on the machine; but equally, disappointed in how slow it then is to fulfil the outstanding merge work. I agree with your original assessment, that not everybody will want deferrable timer, the way it is working at present.

Quoted 4 lines

I expect that can be fixed, partly by doing more work on wakeup from a deferred timer, according to how long it has been deferred; and partly by not deferring on idle until two passes of the list have been completed. But that's easier said than done, and might turn out to

So why not have the timer cancel itself when there is no more work to do and start itself up again when there's work added?

Quoted 30 lines · Hugh Dickins

Quoted 12 lines · Chintan Pandya

KSM thread to scan pages is scheduled on definite timeout. That wakes up CPU from idle state and hence may affect the power consumption. Provide an optional support to use deferrable timer which suites low-power use-cases.

Typically, on our setup we observed, 10% less power consumption with some use-cases in which CPU goes to power collapse frequently. For example, playing audio on Soc which has HW based Audio encoder/decoder, CPU remains idle for longer duration of time. This idle state will save significant CPU power consumption if KSM don't wakes them up periodically.

Quoted 5 lines

Note that, deferrable timers won't be deferred if any CPU is active and not in IDLE state.

By default, deferrable timers is enabled. To disable deferrable timers,

$ echo 0 > /sys/kernel/mm/ksm/deferrable_timer

I have now experimented. And, much as I wanted to eliminate the tunable, and just have deferrable timers on, I have come right back to your original position.

I was impressed by how quiet ksmd goes when there's nothing much happening on the machine; but equally, disappointed in how slow it then is to fulfil the outstanding merge work. I agree with your original assessment, that not everybody will want deferrable timer, the way it is working at present.

Quoted 4 lines

I expect that can be fixed, partly by doing more work on wakeup from a deferred timer, according to how long it has been deferred; and partly by not deferring on idle until two passes of the list have been completed. But that's easier said than done, and might turn out to

So why not have the timer cancel itself when there is no more work to do and start itself up again when there's work added?

Quoted 3 lines

But fixing that might require ksm hooks in hot locations where nobody else would want them: I'm rather hoping we can strike a good enough balance with your deferrable timer, that nobody will need any better.

Well, yes, but... how do we know when there is no more work to do?

Thomas has given reason why KSM might simply fail to do its job if we rely on the deferrable timer. So I've tried another approach, patch below; but I do not expect you to jump for joy at the sight of it!

I've tried to minimize the offensive KSM hook in context_switch(). Why place it there, rather than do something near profile_tick() or account_process_tick()? Because KSM is aware of mms not tasks, and context_switch() should have the next mm cachelines hot (if not, a slight regrouping in mm_struct should do it); whereas I can find no reference whatever to mm_struct in kernel/time, so hooking to KSM from there would drag in another few cachelines every tick.

(Another approach would be to set up KSM hint faulting, along the lines of NUMA hint faulting. Not a path I'm keen to go down.)

I'm not thrilled with this patch, I think it's somewhat defective in several ways. But maybe in practice it will prove good enough, and if so then I'd rather not waste effort on complicating it.

My own testing is not realistic, nor representative of real KSM users; and I have no idea what values of pages_to_scan and sleep_millisecs people really use (and those may make quite a difference to how well it works).

Chintan, even if the scheduler guys turn out to hate it, please would you give the patch below a try, to see how well it works in your environment, whether it seems to go better or worse than your own patch.

If it works well enough for you, maybe we can come up with ideas to make it more palatable. I do think your issue is an important one to fix, one way or another.

Thanks,
Hugh

[PATCH] ksm: avoid periodic wakeup while mergeable mms are quiet

Description yet to be written!

Reported-by Chintan Pandya <[email protected]>

Not-Signed-off-by Hugh Dickins <[email protected]>

---

 include/linux/ksm.h   |   14 +++++++++++
 include/linux/sched.h |    1 
 kernel/sched/core.c   |    9 ++++++-
 mm/ksm.c              |   50 ++++++++++++++++++++++++++++------------
 4 files changed, 58 insertions(+), 16 deletions(-)

include/linux/ksm.h +4 -0

+{
+	return NULL;
+}
+
 static inline int PageKsm(struct page *page)
 {
 	return 0;

kernel/sched/core.c +1 -0

@@ -2304,6 +2305,7 @@ context_switch(struct rq *rq, struct tas
 	       struct task_struct *next)
 {
 	struct mm_struct *mm, *oldmm;
+	wait_queue_head_t *wake_ksm = NULL;
 
 	prepare_task_switch(rq, prev, next);
 

Quoted 1 line

Well, yes, but... how do we know when there is no more work to do?

Yeah, I figured that out _after_ I send that email..

Quoted 2 lines

Thomas has given reason why KSM might simply fail to do its job if we rely on the deferrable timer.

Quoted 8 lines

Chintan, even if the scheduler guys turn out to hate it, please would you give the patch below a try, to see how well it works in your environment, whether it seems to go better or worse than your own patch.

If it works well enough for you, maybe we can come up with ideas to make it more palatable. I do think your issue is an important one to fix, one way or another.

Quoted 8 lines

Thanks,
Hugh

[PATCH] ksm: avoid periodic wakeup while mergeable mms are quiet

Description yet to be written!

Quoted 7 lines

Reported-by Chintan Pandya<[email protected]>

Not-Signed-off-by Hugh Dickins<[email protected]>

Quoted 3 lines

So looking at Hughs test results I'm quite sure that the deferrable timer is just another tunable bandaid with dubious value and the potential of predictable bug/regresssion reports.

Hello All,

Before its too late to discuss this basic question, allow me to share my view on the deferrable timer approach.

I believe KSM at this point is tunable with predictable outcomes. When it will get triggered, how many pages it will scan etc. This aggression control is with user who can implement any complex logic based on its own flashy parameters. Along the same line, I was seeing this deferrable timer knob.

Here I was hoping the same level of predictability with this knob. Kernel still won't do smart work and user is free to play smart/complex with the knob. I believe that there are many use-cases where a single strategy of "to make KSM perform better while saving power at the same time" may not work. So,

Quoted 8 lines · Hugh Dickins

Quoted 1 line

Well, yes, but... how do we know when there is no more work to do?

Yeah, I figured that out _after_ I send that email..

Quoted 2 lines

Thomas has given reason why KSM might simply fail to do its job if we rely on the deferrable timer.

With deferrable timer, KSM thread will be scheduled on the 'active' CPU at that very same time. Yes, I understood from Thomas's clarification that if that very CPU goes IDLE, KSM task will get deferred even if at the timeout, we have some CPUs running. I think, this situation can be avoided potentially very small timeout value (?). But in totality, this is where KSM will be idle for sure, may be that is unwanted.

Quoted 9 lines

Quoted 9 lines

Chintan, even if the scheduler guys turn out to hate it, please would you give the patch below a try, to see how well it works in your environment, whether it seems to go better or worse than your own patch.

If it works well enough for you, maybe we can come up with ideas to make it more palatable. I do think your issue is an important one to fix, one way or another.

It is taking a little more time for me to grasp your change :)

So, after Peter's comment, do you want me to try this out

or you are looking forward for even better idea ?

BTW, if deferrable timer patch gets any green signal, I will publish new patch with your comments on v4.

Quoted 11 lines

Quoted 11 lines

Thanks,
Hugh

[PATCH] ksm: avoid periodic wakeup while mergeable mms are quiet

Description yet to be written!

Reported-by Chintan Pandya<[email protected]>

Not-Signed-off-by Hugh Dickins<[email protected]>

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

So looking at Hughs test results I'm quite sure that the deferrable timer is just another tunable bandaid with dubious value and the potential of predictable bug/regresssion reports.

Here I am naive in understanding the obvious disadvantages of 'one more knob'. And hence was inclined towards deferrable timer knob. Thomas, could you explain what kind of bug/regression you foresee with such approach ?

Signature · 4 lines

Chintan Pandya

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

Quoted 3 lines · Hugh Dickins

include/linux/ksm.h +0 -0

include/linux/ksm.h static inline void ksm_exit(struct mm_st · line 99

Quoted 5 lines

include/linux/ksm.h +1 -0

@@ -87,6 +96,11 @@ static inline void ksm_exit(struct mm_st
 {
 }
 
+static inline wait_queue_head_t *ksm_switch(struct mm_struct *mm)

s/ksm_switch/__&/

Quoted 20 lines

include/linux/ksm.h +4 -0

+{
+	return NULL;
+}
+
 static inline int PageKsm(struct page *page)
 {
 	return 0;

kernel/sched/core.c +1 -0

@@ -2304,6 +2305,7 @@ context_switch(struct rq *rq, struct tas
 	       struct task_struct *next)
 {
 	struct mm_struct *mm, *oldmm;
+	wait_queue_head_t *wake_ksm = NULL;
 
 	prepare_task_switch(rq, prev, next);
 

kernel/sched/core.c context_switch(struct rq *rq, struct tas · line 2327

Quoted 8 lines

kernel/sched/core.c +2 -1

@@ -2320,8 +2322,10 @@ context_switch(struct rq *rq, struct tas
 		next->active_mm = oldmm;
 		atomic_inc(&oldmm->mm_count);
 		enter_lazy_tlb(oldmm, next);
-	} else
+	} else {
 		switch_mm(oldmm, mm, next);
+		wake_ksm = ksm_switch(mm);

Is this the right mm?

We've just switched the stack,

so we're looing at next->mm when we switched away from current. That might not exist anymore.

kernel/sched/core.c context_switch(struct rq *rq, struct tas · line 2358

Quoted 13 lines

kernel/sched/core.c +4 -0

+	}
 
 	if (!prev->mm) {
 		prev->active_mm = NULL;

@@ -2348,6 +2352,9 @@ context_switch(struct rq *rq, struct tas
 	 * frame will be invalid.
 	 */
 	finish_task_switch(this_rq(), prev);
+
+	if (wake_ksm)
+		wake_up_interruptible(wake_ksm);
 }

Quite horrible for sure. I really hate seeing KSM cruft all the way down

here. Can't we create a new (timer) infrastructure that does the right thing? Surely this isn't the only such case.

I know both RCU and some NOHZ_FULL muck already track when the system is completely idle. This is yet another case of that.

Quoted 4 lines · Peter Zijlstra

kernel/sched/core.c context_switch(struct rq *rq, struct tas · line 2327

Quoted 2 lines

kernel/sched/core.c +1 -0

 		switch_mm(oldmm, mm, next);
+		wake_ksm = ksm_switch(mm);

Is this the right mm?

It's next->mm, that's the one I intended (though the patch might be equally workable using prev->mm instead: given free rein, I'd have opted for hooking into both prev and next, but free rein is definitely not what should be granted around here!).

Quoted 1 line

We've just switched the stack,

I thought that came in switch_to() a few lines further down, but don't think it matters for this.

Quoted 2 lines

so we're looing at next->mm when we switched away from current. That might not exist anymore.

I fail to see how that can be. Looking at the x86 switch_mm(), I can see it referencing (unsurprisingly!) both old and new mms at this point, and no reference to an mm is dropped before the ksm_switch(). oldmm (there called mm) is mmdropped later in finish_task_switch().

So if this were switch_to(), we'd see next->mm as before the last context switch. And since that switch fully happened, it would also already have done the finish_task_switch() -> mmdrop().

Quoted 1 line

Quite horrible for sure. I really hate seeing KSM cruft all the way down

Yes, I expected that, and I would certainly feel the same way.

And even worse, imagine if this were successful, we might come along and ask to do something similar for khugepaged. Though if it comes to that, I'm sure we would generalize into one hook which does not say "ksm" or "khugepaged" on it, but would still a present a single unlikely flag to be tested at this level. Maybe you would even prefer the generalized version, but I don't want to complicate the prototype yet.

If it weren't for the "we already have the mm cachelines here" argument, I by now feel fairly sure that I would be going for hooking into timer tick instead (where Thomas could then express his horror!).

Do you think I should just forget about cacheline micro-optimizations and go in that direction instead?

Quoted 2 lines

here. Can't we create a new (timer) infrastructure that does the right thing? Surely this isn't the only such case.

A sleep-walking timer, that goes to sleep in one bed, but may wake in another; and defers while beds are empty? I'd be happy to try using that for KSM if it already existed, and no doubt Chintan would too

But I don't think KSM presents a very good case for developing it. I think KSM's use of a sleep_millisecs timer is really just an apology for the amount of often wasted work that it does, and dates from before we niced it down 5. I prefer the idea of a KSM which waits on activity amongst the restricted set of tasks it is tracking: as this patch tries.

But my preference may be naive: doing lots of unnecessary work doesn't matter as much as waking cpus from deep sleep.

Does it make sense to drive both KSM and khugepage the same way we drive the numa scanning? It has the benefit of getting rid of these threads, which pushes the work into the right accountable context (the task its doing the scanning for) and makes the scanning frequency depend on the actual task activity.

Quoted 2 lines

here. Can't we create a new (timer) infrastructure that does the right thing? Surely this isn't the only such case.

A sleep-walking timer, that goes to sleep in one bed, but may wake in another; and defers while beds are empty? I'd be happy to try using that for KSM if it already existed, and no doubt Chintan would too

But I don't think KSM presents a very good case for developing it. I think KSM's use of a sleep_millisecs timer is really just an apology for the amount of often wasted work that it does, and dates from before we niced it down 5. I prefer the idea of a KSM which waits on activity amongst the restricted set of tasks it is tracking: as this patch tries.

But my preference may be naive: doing lots of unnecessary work doesn't matter as much as waking cpus from deep sleep.

Quoted 4 lines

I know both RCU and some NOHZ_FULL muck already track when the system is completely idle. This is yet another case of that.

Hugh

I don't mean to divert the thread too much. But just one suggestion offered by Harshad.

Why can't we stop invoking more of a KSM scanner thread when we are saturating from savings ? But again, to check whether savings are saturated or not, we may still want to rely upon timers and we have to wake the CPUs up from IDLE state.