linux-kernel - Re: [patch V3 8/8] mm: vmstat_refresh: avoid queueing work item if cpu stats are clean

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2108251121260.317297@gentwo.de>
Date:   Wed, 25 Aug 2021 11:30:43 +0200 (CEST)
From:   Christoph Lameter <cl@...two.de>
To:     Marcelo Tosatti <mtosatti@...hat.com>
cc:     linux-kernel@...r.kernel.org, Nitesh Lal <nilal@...hat.com>,
        Nicolas Saenz Julienne <nsaenzju@...hat.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Alex Belits <abelits@...its.com>, Peter Xu <peterx@...hat.com>
Subject: Re: [patch V3 8/8] mm: vmstat_refresh: avoid queueing work item if
 cpu stats are clean

On Tue, 24 Aug 2021, Marcelo Tosatti wrote:

> It is not necessary to queue work item to run refresh_vm_stats
> on a remote CPU if that CPU has no dirty stats and no per-CPU
> allocations for remote nodes.

The issue in the past was whether the effort to check is adding overhead
that is comparable to run refresh_vm_stats. YMMV.

> This fixes sosreport hang (which uses vmstat_refresh) with
> spinning SCHED_FIFO process.

Ughhh.. SCHED_FIFO is evil....

>  #ifdef CONFIG_PROC_FS
> +static bool need_drain_remote_zones(int cpu)

Well this is not related to vm stats but per cpu pages of the page
allocator. Maybe call this need_drain_remote_pcp or something?

> @@ -1860,8 +1885,12 @@ int vmstat_refresh(struct ctl_table *tab
>  		   void *buffer, size_t *lenp, loff_t *ppos)
>  {
>  	long val;
> -	int err;
> -	int i;
> +	int i, cpu;
> +	struct work_struct __percpu *works;
> +
> +	works = alloc_percpu(struct work_struct);

Do malloc instead? Using the percpu allocator frequently in a function to
allocator temporary variables can cause needless fragmentation there. The
percpu allocator does not have the frag management features of the slab
allocators.