[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20220817190136.096849758@redhat.com>
Date: Wed, 17 Aug 2022 16:01:36 -0300
From: Marcelo Tosatti <mtosatti@...hat.com>
To: atomlin@...hat.com, frederic@...nel.org
Cc: cl@...ux.com, tglx@...utronix.de, mingo@...nel.org,
peterz@...radead.org, pauld@...hat.com, neelx@...hat.com,
oleksandr@...alenko.name, linux-kernel@...r.kernel.org,
linux-mm@...ck.org
Subject: [patch 0/3] tick/sched: Ensure quiet_vmstat() is called when the idle tick was stopped too
This patchset contains enhancements on top of Aaron's -v6 of the series
(see the changelog below).
It fixes the following problems two problems:
1) 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()).
2) With a SCHED_FIFO task that busy loops on a given CPU,
and kworker for that CPU at SCHED_OTHER priority, queuing
work to sync per-vmstats will either cause that work
to never execute, or stalld boosts kworker priority which
causes a latency violation.
Follows the v6 cover letter, with updated changelog. The numbers, for
the test program attached at the end of this cover letter, executed
inside a KVM VM, are:
Vanilla Patch
cycles per idle loop 151858 153258 (+1.0%)
cycles per syscall 8461 8690 (+2.6%)
--------
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 v6 [6]:
- sync vmstats independently of whether vmstat_update work is queued or not
- clean vmstat_dirty before differential sync loop
- cancel pending work if tick stopped
- do not queue work to remote CPU if tick stopped
Changes since v5 [3]:
- Introduced __tick_nohz_user_enter_prepare()
- Switched to EXPORT_SYMBOL_GPL()
Changes since v4 [4]:
- Moved vmstat_dirty specific changes into a separate patch
(Marcelo Tosatti)
Changes since v3 [5]:
- 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/20220204173554.763888172@fedora.localdomain/
[2]: https://pastebin.com/8AtzSAuK
[3]: https://lore.kernel.org/lkml/20220801234258.134609-1-atomlin@redhat.com/
[4]: https://lore.kernel.org/lkml/20220621172207.1501641-1-atomlin@redhat.com/
[5]: https://lore.kernel.org/lkml/20220422193647.3808657-1-atomlin@redhat.com/
[6]: https://lore.kernel.org/linux-mm/20220808194820.676246-1-atomlin@redhat.com/
include/linux/tick.h | 5 +++--
kernel/time/tick-sched.c | 19 ++++++++++++++++++-
mm/vmstat.c | 74 ++++++++++++++++++++++++++++++++++++--------------------------------------
3 files changed, 57 insertions(+), 41 deletions(-)
--- test-vmstat-overhead.c ---
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <unistd.h>
#include <string.h>
typedef unsigned long long cycles_t;
typedef unsigned long long usecs_t;
typedef unsigned long long u64;
#ifdef __x86_64__
#define DECLARE_ARGS(val, low, high) unsigned long low, high
#define EAX_EDX_VAL(val, low, high) ((low) | ((u64)(high) << 32))
#define EAX_EDX_ARGS(val, low, high) "a" (low), "d" (high)
#define EAX_EDX_RET(val, low, high) "=a" (low), "=d" (high)
#else
#define DECLARE_ARGS(val, low, high) unsigned long long val
#define EAX_EDX_VAL(val, low, high) (val)
#define EAX_EDX_ARGS(val, low, high) "A" (val)
#define EAX_EDX_RET(val, low, high) "=A" (val)
#endif
static inline unsigned long long __rdtscll(void)
{
DECLARE_ARGS(val, low, high);
asm volatile("cpuid; rdtsc" : EAX_EDX_RET(val, low, high));
return EAX_EDX_VAL(val, low, high);
}
#define rdtscll(val) do { (val) = __rdtscll(); } while (0)
#define NRSYSCALLS 30000
#define NRSLEEPS 100000
void main(int argc, char *argv[])
{
unsigned long a, b, cycles;
int i, syscall = 0;
void *page = malloc(4096);
if (mlock(page, 4096))
perror("mlock");
if (munlock(page, 4096))
perror("munlock");
if (argc != 2) {
printf("usage: %s {idle,syscall}\n", argv[0]);
exit(1);
}
rdtscll(a);
if (strncmp("idle", argv[1], 4) == 0)
syscall = 0;
else if (strncmp("syscall", argv[1], 7) == 0)
syscall = 1;
else {
printf("usage: %s {idle,syscall}\n", argv[0]);
exit(1);
}
if (syscall == 1) {
for (i = 0; i < NRSYSCALLS; i++) {
if (mlock(page, 4096))
perror("mlock");
if (munlock(page, 4096))
perror("munlock");
}
} else {
for (i = 0; i < NRSLEEPS; i++)
usleep(10);
}
rdtscll(b);
cycles = b - a;
if (syscall == 1)
printf("cycles per syscall: %d\n", (b-a)/(NRSYSCALLS*2));
else
printf("cycles per idle loop: %d\n", (b-a)/NRSLEEPS);
}
Powered by blists - more mailing lists