LinuxLists.cc - [PATCH v5 0/2] tick/sched: Ensure quiet_vmstat() is called when the idle tick was stopped too

2022-08-01 23:49:09

Subject: [PATCH v5 0/2] tick/sched: Ensure quiet_vmstat() is called when the idle tick was stopped too

Hi Frederic and Marcelo,

I have incorporated an idea from Marcelo's patch [1] where a CPU-specific
variable is used to indicate if a vmstat differential/or imbalance is
present for a given CPU. So, at the appropriate time, vmstat processing can
be initiated. The hope is that this particular approach is "cheaper" when
compared to need_update() - used currently; in the context of nohz_full and
the scheduling-clock tick being stopped, we would now with this patch,
check if a CPU-specific vmstat imbalance is present before exiting
user-mode (see tick_nohz_user_enter_prepare()).

This trivial test program [2] was used to determine the somewhat impact
under vanilla and with the proposed changes; mlock(2) and munlock(2) was
used solely to modify vmstat item 'NR_MLOCK'. The following is an average
count of CPU-cycles across the aforementioned system calls and the idle
loop, respectively. I believe these results are negligible:

Modified | Vanilla
|
cycles per syscall: 7399 | cycles per syscall: 4150
cycles per idle loop: 141048 | cycles per idle loop: 144730
|

Any feedback would be appreciated. Thanks.

Changes since v4 [3]:

- Moved vmstat_dirty specific changes into a separate patch
(Marcelo Tosatti)

Changes since v3 [4]:

- Used EXPORT_SYMBOL() on tick_nohz_user_enter_prepare()
- Replaced need_update()
- Introduced CPU-specific variable namely vmstat_dirty
and mark_vmstat_dirty()

[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://pastebin.com/8AtzSAuK
[3]: https://lore.kernel.org/lkml/[email protected]/
[4]: https://lore.kernel.org/lkml/[email protected]/

Aaron Tomlin (2):
mm/vmstat: Use per cpu variable to track a vmstat discrepancy
tick/sched: Ensure quiet_vmstat() is called when the idle tick was
stopped too

include/linux/tick.h | 9 ++------
kernel/time/tick-sched.c | 19 ++++++++++++++++-
mm/vmstat.c | 46 +++++++++++++---------------------------
3 files changed, 35 insertions(+), 39 deletions(-)

--
2.37.1

2022-08-01 23:57:46

by Aaron Tomlin

[permalink] [raw]

Subject: [PATCH v5 2/2] tick/sched: Ensure quiet_vmstat() is called when the idle tick was stopped too

In the context of the idle task and an adaptive-tick mode/or a nohz_full
CPU, quiet_vmstat() can be called: before stopping the idle tick,
entering an idle state and on exit. In particular, for the latter case,
when the idle task is required to reschedule, the idle tick can remain
stopped and the timer expiration time endless i.e., KTIME_MAX. Now,
indeed before a nohz_full CPU enters an idle state, CPU-specific vmstat
counters should be processed to ensure the respective values have been
reset and folded into the zone specific 'vm_stat[]'. That being said, it
can only occur when: the idle tick was previously stopped, and
reprogramming of the timer is not required.

A customer provided some evidence which indicates that the idle tick was
stopped; albeit, CPU-specific vmstat counters still remained populated.
Thus one can only assume quiet_vmstat() was not invoked on return to the
idle loop.

If I understand correctly, I suspect this divergence might erroneously
prevent a reclaim attempt by kswapd. If the number of zone specific free
pages are below their per-cpu drift value then
zone_page_state_snapshot() is used to compute a more accurate view of
the aforementioned statistic. Thus any task blocked on the NUMA node
specific pfmemalloc_wait queue will be unable to make significant
progress via direct reclaim unless it is killed after being woken up by
kswapd (see throttle_direct_reclaim()).

Consider the following theoretical scenario:

1. CPU Y migrated running task A to CPU X that was
in an idle state i.e. waiting for an IRQ - not
polling; marked the current task on CPU X to
need/or require a reschedule i.e., set
TIF_NEED_RESCHED and invoked a reschedule IPI to
CPU X (see sched_move_task())

2. CPU X acknowledged the reschedule IPI from CPU Y;
generic idle loop code noticed the
TIF_NEED_RESCHED flag against the idle task and
attempts to exit of the loop and calls the main
scheduler function i.e. __schedule().

Since the idle tick was previously stopped no
scheduling-clock tick would occur.
So, no deferred timers would be handled

3. Post transition to kernel execution Task A
running on CPU Y, indirectly released a few pages
(e.g. see __free_one_page()); CPU Y's
'vm_stat_diff[NR_FREE_PAGES]' was updated and zone
specific 'vm_stat[]' update was deferred as per the
CPU-specific stat threshold

4. Task A does invoke exit(2) and the kernel does
remove the task from the run-queue; the idle task
was selected to execute next since there are no
other runnable tasks assigned to the given CPU
(see pick_next_task() and pick_next_task_idle())

5. On return to the idle loop since the idle tick
was already stopped and can remain so (see [1]
below) e.g. no pending soft IRQs, no attempt is
made to zero and fold CPU Y's vmstat counters
since reprogramming of the scheduling-clock tick
is not required/or needed (see [2])

...
do_idle
{

__current_set_polling()
tick_nohz_idle_enter()

while (!need_resched()) {

local_irq_disable()

...

/* No polling or broadcast event */
cpuidle_idle_call()
{

if (cpuidle_not_available(drv, dev)) {
tick_nohz_idle_stop_tick()
__tick_nohz_idle_stop_tick(this_cpu_ptr(&tick_cpu_sched))
{
int cpu = smp_processor_id()

if (ts->timer_expires_base)
expires = ts->timer_expires
else if (can_stop_idle_tick(cpu, ts))
(1) -------> expires = tick_nohz_next_event(ts, cpu)
else
return

ts->idle_calls++

if (expires > 0LL) {

tick_nohz_stop_tick(ts, cpu)
{

if (ts->tick_stopped && (expires == ts->next_tick)) {
(2) -------> if (tick == KTIME_MAX || ts->next_tick ==
hrtimer_get_expires(&ts->sched_timer))
return
}
...
}

So the idea of with this patch is to ensure refresh_cpu_vm_stats(false) is
called, when it is appropriate, on return to the idle loop when the idle
tick was previously stopped too. Additionally, in the context of
nohz_full, when the scheduling-tick is stopped and before exiting
to user-mode, ensure no CPU-specific vmstat differentials remain.

Signed-off-by: Aaron Tomlin <[email protected]>
---
include/linux/tick.h | 9 ++-------
kernel/time/tick-sched.c | 19 ++++++++++++++++++-
2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index bfd571f18cfd..4c576c9ca0a2 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -11,7 +11,6 @@
#include <linux/context_tracking_state.h>
#include <linux/cpumask.h>
#include <linux/sched.h>
-#include <linux/rcupdate.h>

#ifdef CONFIG_GENERIC_CLOCKEVENTS
extern void __init tick_init(void);
@@ -123,6 +122,8 @@ enum tick_dep_bits {
#define TICK_DEP_MASK_RCU (1 << TICK_DEP_BIT_RCU)
#define TICK_DEP_MASK_RCU_EXP (1 << TICK_DEP_BIT_RCU_EXP)

+void tick_nohz_user_enter_prepare(void);
+
#ifdef CONFIG_NO_HZ_COMMON
extern bool tick_nohz_enabled;
extern bool tick_nohz_tick_stopped(void);
@@ -305,10 +306,4 @@ static inline void tick_nohz_task_switch(void)
__tick_nohz_task_switch();
}

-static inline void tick_nohz_user_enter_prepare(void)
-{
- if (tick_nohz_full_cpu(smp_processor_id()))
- rcu_nocb_flush_deferred_wakeup();
-}
-
#endif
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 30049580cd62..c7c69a974414 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -26,6 +26,7 @@
#include <linux/posix-timers.h>
#include <linux/context_tracking.h>
#include <linux/mm.h>
+#include <linux/rcupdate.h>

#include <asm/irq_regs.h>

@@ -43,6 +44,20 @@ struct tick_sched *tick_get_tick_sched(int cpu)
return &per_cpu(tick_cpu_sched, cpu);
}

+void tick_nohz_user_enter_prepare(void)
+{
+ struct tick_sched *ts;
+
+ if (tick_nohz_full_cpu(smp_processor_id())) {
+ ts = this_cpu_ptr(&tick_cpu_sched);
+
+ if (ts->tick_stopped)
+ quiet_vmstat();
+ rcu_nocb_flush_deferred_wakeup();
+ }
+}
+EXPORT_SYMBOL(tick_nohz_user_enter_prepare);
+
#if defined(CONFIG_NO_HZ_COMMON) || defined(CONFIG_HIGH_RES_TIMERS)
/*
* The time, when the last jiffy update happened. Write access must hold
@@ -890,6 +905,9 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu)
ts->do_timer_last = 0;
}

+ /* Attempt to fold when the idle tick is stopped or not */
+ quiet_vmstat();
+
/* Skip reprogram of event if its not changed */
if (ts->tick_stopped && (expires == ts->next_tick)) {
/* Sanity check: make sure clockevent is actually programmed */
@@ -911,7 +929,6 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu)
*/
if (!ts->tick_stopped) {
calc_load_nohz_start();
- quiet_vmstat();

ts->last_tick = hrtimer_get_expires(&ts->sched_timer);
ts->tick_stopped = 1;
--
2.37.1

2022-08-02 00:53:18

by Aaron Tomlin

[permalink] [raw]

Subject: [PATCH v5 1/2] mm/vmstat: Use per cpu variable to track a vmstat discrepancy

This patch incorporates an idea from Marcelo's patch [1] where a
CPU-specific variable namely vmstat_dirty is used to indicate if
a vmstat imbalance is present for a given CPU. Therefore, at the
appropriate time, we can fold all the remaining differentials.

[1]: https://lore.kernel.org/lkml/[email protected]/

Signed-off-by: Aaron Tomlin <[email protected]>
---
mm/vmstat.c | 46 +++++++++++++++-------------------------------
1 file changed, 15 insertions(+), 31 deletions(-)

diff --git a/mm/vmstat.c b/mm/vmstat.c
index 373d2730fcf2..51564b7c85fe 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -195,6 +195,12 @@ void fold_vm_numa_events(void)
#endif

#ifdef CONFIG_SMP
+static DEFINE_PER_CPU_ALIGNED(bool, vmstat_dirty);
+
+static inline void mark_vmstat_dirty(void)
+{
+ this_cpu_write(vmstat_dirty, true);
+}

int calculate_pressure_threshold(struct zone *zone)
{
@@ -367,6 +373,7 @@ void __mod_zone_page_state(struct zone *zone, enum zone_stat_item item,
x = 0;
}
__this_cpu_write(*p, x);
+ mark_vmstat_dirty();

if (IS_ENABLED(CONFIG_PREEMPT_RT))
preempt_enable();
@@ -405,6 +412,7 @@ void __mod_node_page_state(struct pglist_data *pgdat, enum node_stat_item item,
x = 0;
}
__this_cpu_write(*p, x);
+ mark_vmstat_dirty();

if (IS_ENABLED(CONFIG_PREEMPT_RT))
preempt_enable();
@@ -603,6 +611,7 @@ static inline void mod_zone_state(struct zone *zone,

if (z)
zone_page_state_add(z, zone, item);
+ mark_vmstat_dirty();
}

void mod_zone_page_state(struct zone *zone, enum zone_stat_item item,
@@ -671,6 +680,7 @@ static inline void mod_node_state(struct pglist_data *pgdat,

if (z)
node_page_state_add(z, pgdat, item);
+ mark_vmstat_dirty();
}

void mod_node_page_state(struct pglist_data *pgdat, enum node_stat_item item,
@@ -1873,6 +1883,7 @@ int sysctl_stat_interval __read_mostly = HZ;
static void refresh_vm_stats(struct work_struct *work)
{
refresh_cpu_vm_stats(true);
+ this_cpu_write(vmstat_dirty, false);
}

int vmstat_refresh(struct ctl_table *table, int write,
@@ -1937,6 +1948,7 @@ int vmstat_refresh(struct ctl_table *table, int write,
static void vmstat_update(struct work_struct *w)
{
if (refresh_cpu_vm_stats(true)) {
+ this_cpu_write(vmstat_dirty, false);
/*
* Counters were updated so we expect more updates
* to occur in the future. Keep on running the
@@ -1948,35 +1960,6 @@ static void vmstat_update(struct work_struct *w)
}
}

-/*
- * Check if the diffs for a certain cpu indicate that
- * an update is needed.
- */
-static bool need_update(int cpu)
-{
- pg_data_t *last_pgdat = NULL;
- struct zone *zone;
-
- for_each_populated_zone(zone) {
- struct per_cpu_zonestat *pzstats = per_cpu_ptr(zone->per_cpu_zonestats, cpu);
- struct per_cpu_nodestat *n;
-
- /*
- * The fast way of checking if there are any vmstat diffs.
- */
- if (memchr_inv(pzstats->vm_stat_diff, 0, sizeof(pzstats->vm_stat_diff)))
- return true;
-
- if (last_pgdat == zone->zone_pgdat)
- continue;
- last_pgdat = zone->zone_pgdat;
- n = per_cpu_ptr(zone->zone_pgdat->per_cpu_nodestats, cpu);
- if (memchr_inv(n->vm_node_stat_diff, 0, sizeof(n->vm_node_stat_diff)))
- return true;
- }
- return false;
-}
-
/*
* Switch off vmstat processing and then fold all the remaining differentials
* until the diffs stay at zero. The function is used by NOHZ and can only be
@@ -1990,7 +1973,7 @@ void quiet_vmstat(void)
if (!delayed_work_pending(this_cpu_ptr(&vmstat_work)))
return;

- if (!need_update(smp_processor_id()))
+ if (!__this_cpu_read(vmstat_dirty))
return;

/*
@@ -2000,6 +1983,7 @@ void quiet_vmstat(void)
* vmstat_shepherd will take care about that for us.
*/
refresh_cpu_vm_stats(false);
+ __this_cpu_write(vmstat_dirty, false);
}

/*
@@ -2021,7 +2005,7 @@ static void vmstat_shepherd(struct work_struct *w)
for_each_online_cpu(cpu) {
struct delayed_work *dw = &per_cpu(vmstat_work, cpu);

- if (!delayed_work_pending(dw) && need_update(cpu))
+ if (!delayed_work_pending(dw) && per_cpu(vmstat_dirty, cpu))
queue_delayed_work_on(cpu, mm_percpu_wq, dw, 0);

cond_resched();
--
2.37.1

2022-08-02 22:43:15

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] tick/sched: Ensure quiet_vmstat() is called when the idle tick was stopped too

Hi Aaron,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.19 next-20220728]
[cannot apply to tip/timers/nohz]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Aaron-Tomlin/tick-sched-Ensure-quiet_vmstat-is-called-when-the-idle-tick-was-stopped-too/20220802-074341
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 9de1f9c8ca5100a02a2e271bdbde36202e251b4b
config: i386-randconfig-a002-20220801 (https://download.01.org/0day-ci/archive/20220803/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/a0d3b9fe31484c4c44c430d10d0b60e2e0551525
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Aaron-Tomlin/tick-sched-Ensure-quiet_vmstat-is-called-when-the-idle-tick-was-stopped-too/20220802-074341
git checkout a0d3b9fe31484c4c44c430d10d0b60e2e0551525
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: tick_nohz_user_enter_prepare
>>> referenced by common.c
>>> entry/common.o:(exit_to_user_mode_prepare) in archive kernel/built-in.a
>>> referenced by common.c
>>> entry/common.o:(exit_to_user_mode_prepare) in archive kernel/built-in.a

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-08-03 05:48:45

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] tick/sched: Ensure quiet_vmstat() is called when the idle tick was stopped too

Hi Aaron,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.19 next-20220802]
[cannot apply to tip/timers/nohz]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Aaron-Tomlin/tick-sched-Ensure-quiet_vmstat-is-called-when-the-idle-tick-was-stopped-too/20220802-074341
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 9de1f9c8ca5100a02a2e271bdbde36202e251b4b
config: x86_64-randconfig-a016-20220801 (https://download.01.org/0day-ci/archive/20220803/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/a0d3b9fe31484c4c44c430d10d0b60e2e0551525
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Aaron-Tomlin/tick-sched-Ensure-quiet_vmstat-is-called-when-the-idle-tick-was-stopped-too/20220802-074341
git checkout a0d3b9fe31484c4c44c430d10d0b60e2e0551525
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

ld: vmlinux.o: in function `exit_to_user_mode_loop':
>> kernel/entry/common.c:182: undefined reference to `tick_nohz_user_enter_prepare'
ld: vmlinux.o: in function `exit_to_user_mode_prepare':
kernel/entry/common.c:198: undefined reference to `tick_nohz_user_enter_prepare'

vim +182 kernel/entry/common.c

a9f3a74a29af095 Thomas Gleixner 2020-07-22 144
a9f3a74a29af095 Thomas Gleixner 2020-07-22 145 static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
a9f3a74a29af095 Thomas Gleixner 2020-07-22 146 unsigned long ti_work)
a9f3a74a29af095 Thomas Gleixner 2020-07-22 147 {
a9f3a74a29af095 Thomas Gleixner 2020-07-22 148 /*
a9f3a74a29af095 Thomas Gleixner 2020-07-22 149 * Before returning to user space ensure that all pending work
a9f3a74a29af095 Thomas Gleixner 2020-07-22 150 * items have been completed.
a9f3a74a29af095 Thomas Gleixner 2020-07-22 151 */
a9f3a74a29af095 Thomas Gleixner 2020-07-22 152 while (ti_work & EXIT_TO_USER_MODE_WORK) {
a9f3a74a29af095 Thomas Gleixner 2020-07-22 153
a9f3a74a29af095 Thomas Gleixner 2020-07-22 154 local_irq_enable_exit_to_user(ti_work);
a9f3a74a29af095 Thomas Gleixner 2020-07-22 155
a9f3a74a29af095 Thomas Gleixner 2020-07-22 156 if (ti_work & _TIF_NEED_RESCHED)
a9f3a74a29af095 Thomas Gleixner 2020-07-22 157 schedule();
a9f3a74a29af095 Thomas Gleixner 2020-07-22 158
a9f3a74a29af095 Thomas Gleixner 2020-07-22 159 if (ti_work & _TIF_UPROBE)
a9f3a74a29af095 Thomas Gleixner 2020-07-22 160 uprobe_notify_resume(regs);
a9f3a74a29af095 Thomas Gleixner 2020-07-22 161
a9f3a74a29af095 Thomas Gleixner 2020-07-22 162 if (ti_work & _TIF_PATCH_PENDING)
a9f3a74a29af095 Thomas Gleixner 2020-07-22 163 klp_update_patch_state(current);
a9f3a74a29af095 Thomas Gleixner 2020-07-22 164
12db8b690010ccf Jens Axboe 2020-10-26 165 if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
8ba62d37949e248 Eric W. Biederman 2022-02-09 166 arch_do_signal_or_restart(regs);
a9f3a74a29af095 Thomas Gleixner 2020-07-22 167
a68de80f61f6af3 Sean Christopherson 2021-09-01 168 if (ti_work & _TIF_NOTIFY_RESUME)
03248addadf1a5e Eric W. Biederman 2022-02-09 169 resume_user_mode_work(regs);
a9f3a74a29af095 Thomas Gleixner 2020-07-22 170
a9f3a74a29af095 Thomas Gleixner 2020-07-22 171 /* Architecture specific TIF work */
a9f3a74a29af095 Thomas Gleixner 2020-07-22 172 arch_exit_to_user_mode_work(regs, ti_work);
a9f3a74a29af095 Thomas Gleixner 2020-07-22 173
a9f3a74a29af095 Thomas Gleixner 2020-07-22 174 /*
a9f3a74a29af095 Thomas Gleixner 2020-07-22 175 * Disable interrupts and reevaluate the work flags as they
a9f3a74a29af095 Thomas Gleixner 2020-07-22 176 * might have changed while interrupts and preemption was
a9f3a74a29af095 Thomas Gleixner 2020-07-22 177 * enabled above.
a9f3a74a29af095 Thomas Gleixner 2020-07-22 178 */
a9f3a74a29af095 Thomas Gleixner 2020-07-22 179 local_irq_disable_exit_to_user();
47b8ff194c1fd73 Frederic Weisbecker 2021-02-01 180
47b8ff194c1fd73 Frederic Weisbecker 2021-02-01 181 /* Check if any of the above work has queued a deferred wakeup */
f268c3737ecaefc Frederic Weisbecker 2021-05-27 @182 tick_nohz_user_enter_prepare();
47b8ff194c1fd73 Frederic Weisbecker 2021-02-01 183
6ce895128b3bff7 Mark Rutland 2021-11-29 184 ti_work = read_thread_flags();
a9f3a74a29af095 Thomas Gleixner 2020-07-22 185 }
a9f3a74a29af095 Thomas Gleixner 2020-07-22 186
a9f3a74a29af095 Thomas Gleixner 2020-07-22 187 /* Return the latest work state for arch_exit_to_user_mode() */
a9f3a74a29af095 Thomas Gleixner 2020-07-22 188 return ti_work;
a9f3a74a29af095 Thomas Gleixner 2020-07-22 189 }
a9f3a74a29af095 Thomas Gleixner 2020-07-22 190

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-08-03 07:00:50

by kernel test robot

[permalink] [raw]