2022-03-16 19:25:38

by Marcelo Tosatti

[permalink] [raw]
Subject: [patch v12 07/13] task isolation: sync vmstats conditional on changes

Rather than syncing VM-stats on every return to userspace
(or VM-entry), keep track of changes through a per-CPU bool.

This improves performance when enabling task isolated
for vcpu VMs.

Signed-off-by: Marcelo Tosatti <[email protected]>

---
v11:
- Add TIF_TASK_ISOL bit to thread info flags and use it
to decide whether to perform task isolation work on
return to userspace

include/linux/vmstat.h | 13 ++++++++++++-
mm/vmstat.c | 29 ++++++++++++++++++++++++++++-
2 files changed, 40 insertions(+), 2 deletions(-)

Index: linux-2.6/include/linux/vmstat.h
===================================================================
--- linux-2.6.orig/include/linux/vmstat.h
+++ linux-2.6/include/linux/vmstat.h
@@ -22,7 +22,18 @@ int sysctl_vm_numa_stat_handler(struct c
#endif

#if defined(CONFIG_SMP) && defined(CONFIG_TASK_ISOLATION)
-void sync_vmstat(void);
+DECLARE_PER_CPU_ALIGNED(bool, vmstat_dirty);
+
+extern struct static_key vmstat_sync_enabled;
+
+void __sync_vmstat(void);
+static inline void sync_vmstat(void)
+{
+ if (static_key_false(&vmstat_sync_enabled))
+ __sync_vmstat();
+}
+
+void init_sync_vmstat(void);
#else
static inline void sync_vmstat(void)
{
Index: linux-2.6/mm/vmstat.c
===================================================================
--- linux-2.6.orig/mm/vmstat.c
+++ linux-2.6/mm/vmstat.c
@@ -334,6 +334,31 @@ void set_pgdat_percpu_threshold(pg_data_
}
}

+#ifdef CONFIG_TASK_ISOLATION
+struct static_key vmstat_sync_enabled;
+DEFINE_PER_CPU_ALIGNED(bool, vmstat_dirty);
+
+static inline void mark_vmstat_dirty(void)
+{
+ if (!static_key_false(&vmstat_sync_enabled))
+ return;
+
+ raw_cpu_write(vmstat_dirty, true);
+ set_thread_flag(TIF_TASK_ISOL);
+}
+
+void init_sync_vmstat(void)
+{
+ raw_cpu_write(vmstat_dirty, true);
+ set_thread_flag(TIF_TASK_ISOL);
+}
+EXPORT_SYMBOL_GPL(vmstat_dirty);
+#else
+static inline void mark_vmstat_dirty(void)
+{
+}
+#endif
+
/*
* For use when we know that interrupts are disabled,
* or when we know that preemption is disabled and that
@@ -366,6 +391,7 @@ void __mod_zone_page_state(struct zone *
x = 0;
}
__this_cpu_write(*p, x);
+ mark_vmstat_dirty();

if (IS_ENABLED(CONFIG_PREEMPT_RT))
preempt_enable();
@@ -404,6 +430,7 @@ void __mod_node_page_state(struct pglist
x = 0;
}
__this_cpu_write(*p, x);
+ mark_vmstat_dirty();

if (IS_ENABLED(CONFIG_PREEMPT_RT))
preempt_enable();
@@ -602,6 +629,7 @@ static inline void mod_zone_state(struct

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,
@@ -670,6 +698,7 @@ static inline void mod_node_state(struct

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,
@@ -1087,6 +1116,7 @@ static void fill_contig_page_info(struct
info->free_blocks_suitable += blocks <<
(order - suitable_order);
}
+ mark_vmstat_dirty();
}

/*
@@ -1443,6 +1473,7 @@ static void walk_zones_in_node(struct se
if (!nolock)
spin_unlock_irqrestore(&zone->lock, flags);
}
+ mark_vmstat_dirty();
}
#endif

@@ -1512,6 +1543,7 @@ static void pagetypeinfo_showfree_print(
}
seq_putc(m, '\n');
}
+ mark_vmstat_dirty();
}

/* Print out the free pages at each order for each migatetype */
@@ -1932,6 +1964,7 @@ static void vmstat_update(struct work_st
this_cpu_ptr(&vmstat_work),
round_jiffies_relative(sysctl_stat_interval));
}
+ mark_vmstat_dirty();
}

/*
@@ -2019,13 +2052,14 @@ static void vmstat_shepherd(struct work_
}

#ifdef CONFIG_TASK_ISOLATION
-void sync_vmstat(void)
+void __sync_vmstat(void)
{
int cpu;

cpu = get_cpu();

refresh_cpu_vm_stats(false);
+ raw_cpu_write(vmstat_dirty, false);
put_cpu();

/*



2022-03-17 20:16:37

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [patch v12 07/13] task isolation: sync vmstats conditional on changes

On Tue, Mar 15, 2022 at 12:31:39PM -0300, Marcelo Tosatti wrote:
> Rather than syncing VM-stats on every return to userspace
> (or VM-entry), keep track of changes through a per-CPU bool.
>
> This improves performance when enabling task isolated
> for vcpu VMs.
>
> Signed-off-by: Marcelo Tosatti <[email protected]>
>
> ---
> v11:
> - Add TIF_TASK_ISOL bit to thread info flags and use it
> to decide whether to perform task isolation work on
> return to userspace
>
> include/linux/vmstat.h | 13 ++++++++++++-
> mm/vmstat.c | 29 ++++++++++++++++++++++++++++-
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/include/linux/vmstat.h
> ===================================================================
> --- linux-2.6.orig/include/linux/vmstat.h
> +++ linux-2.6/include/linux/vmstat.h
> @@ -22,7 +22,18 @@ int sysctl_vm_numa_stat_handler(struct c
> #endif
>
> #if defined(CONFIG_SMP) && defined(CONFIG_TASK_ISOLATION)
> -void sync_vmstat(void);
> +DECLARE_PER_CPU_ALIGNED(bool, vmstat_dirty);
> +
> +extern struct static_key vmstat_sync_enabled;
> +
> +void __sync_vmstat(void);
> +static inline void sync_vmstat(void)
> +{
> + if (static_key_false(&vmstat_sync_enabled))
> + __sync_vmstat();
> +}
> +
> +void init_sync_vmstat(void);
> #else
> static inline void sync_vmstat(void)
> {
> Index: linux-2.6/mm/vmstat.c
> ===================================================================
> --- linux-2.6.orig/mm/vmstat.c
> +++ linux-2.6/mm/vmstat.c
> @@ -334,6 +334,31 @@ void set_pgdat_percpu_threshold(pg_data_
> }
> }
>
> +#ifdef CONFIG_TASK_ISOLATION
> +struct static_key vmstat_sync_enabled;
> +DEFINE_PER_CPU_ALIGNED(bool, vmstat_dirty);
> +
> +static inline void mark_vmstat_dirty(void)
> +{
> + if (!static_key_false(&vmstat_sync_enabled))
> + return;
> +
> + raw_cpu_write(vmstat_dirty, true);
> + set_thread_flag(TIF_TASK_ISOL);
> +}
> +
> +void init_sync_vmstat(void)
> +{
> + raw_cpu_write(vmstat_dirty, true);
> + set_thread_flag(TIF_TASK_ISOL);
> +}
> +EXPORT_SYMBOL_GPL(vmstat_dirty);
> +#else
> +static inline void mark_vmstat_dirty(void)
> +{
> +}
> +#endif
> +
> /*
> * For use when we know that interrupts are disabled,
> * or when we know that preemption is disabled and that
> @@ -366,6 +391,7 @@ void __mod_zone_page_state(struct zone *
> x = 0;
> }
> __this_cpu_write(*p, x);
> + mark_vmstat_dirty();
>
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> preempt_enable();
> @@ -404,6 +430,7 @@ void __mod_node_page_state(struct pglist
> x = 0;
> }
> __this_cpu_write(*p, x);
> + mark_vmstat_dirty();
>
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> preempt_enable();
> @@ -602,6 +629,7 @@ static inline void mod_zone_state(struct
>
> 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,
> @@ -670,6 +698,7 @@ static inline void mod_node_state(struct
>
> 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,
> @@ -1087,6 +1116,7 @@ static void fill_contig_page_info(struct
> info->free_blocks_suitable += blocks <<
> (order - suitable_order);
> }
> + mark_vmstat_dirty();
> }
>
> /*
> @@ -1443,6 +1473,7 @@ static void walk_zones_in_node(struct se
> if (!nolock)
> spin_unlock_irqrestore(&zone->lock, flags);
> }
> + mark_vmstat_dirty();
> }
> #endif
>
> @@ -1512,6 +1543,7 @@ static void pagetypeinfo_showfree_print(
> }
> seq_putc(m, '\n');
> }
> + mark_vmstat_dirty();
> }
>
> /* Print out the free pages at each order for each migatetype */
> @@ -1932,6 +1964,7 @@ static void vmstat_update(struct work_st
> this_cpu_ptr(&vmstat_work),
> round_jiffies_relative(sysctl_stat_interval));
> }
> + mark_vmstat_dirty();
> }
>
> /*
> @@ -2019,13 +2052,14 @@ static void vmstat_shepherd(struct work_
> }
>
> #ifdef CONFIG_TASK_ISOLATION
> -void sync_vmstat(void)
> +void __sync_vmstat(void)
> {
> int cpu;
>
> cpu = get_cpu();
>
> refresh_cpu_vm_stats(false);
> + raw_cpu_write(vmstat_dirty, false);

I still see a few raw_cpu_write() here, especially that one should
clearly become __this_cpu_write().

> put_cpu();
>
> /*
>
>

2022-04-27 09:44:48

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch v12 07/13] task isolation: sync vmstats conditional on changes

On Tue, Mar 15 2022 at 12:31, Marcelo Tosatti wrote:
>
> +#ifdef CONFIG_TASK_ISOLATION
> +struct static_key vmstat_sync_enabled;

jump_label.h:

"The use of 'struct static_key' directly, is now DEPRECATED."

> +DEFINE_PER_CPU_ALIGNED(bool, vmstat_dirty);
> +
> +static inline void mark_vmstat_dirty(void)
> +{
> + if (!static_key_false(&vmstat_sync_enabled))
> + return;
> +
> + raw_cpu_write(vmstat_dirty, true);

What's the justification for raw_cpu_write()?

>
> @@ -1512,6 +1543,7 @@ static void pagetypeinfo_showfree_print(
> }
> seq_putc(m, '\n');
> }
> + mark_vmstat_dirty();

Why does a function which just dumps information via /proc/pagetypeinfo
make vmstats dirty?

Thanks,

tglx