Messages in this thread | | | Date | Thu, 17 Mar 2022 15:51:43 +0100 | From | Frederic Weisbecker <> | 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 <mtosatti@redhat.com> > > --- > 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(); > > /* > >
| |