Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 9 Jan 2023 11:12:49 -0300
From:   Marcelo Tosatti <mtosatti@redhat.com>
To:     Hillf Danton <hdanton@sina.com>
Cc:     atomlin@atomlin.com, frederic@kernel.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v13 3/6] mm/vmstat: manage per-CPU stats from CPU context
 when NOHZ full
Message-ID: <Y7wg4bC65mNcpElo@tpad>
References: <20230105125218.031928326@redhat.com>
 <20230106001244.4463-1-hdanton@sina.com>
 <20230106150154.4560-1-hdanton@sina.com>
 <20230107001529.4617-1-hdanton@sina.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230107001529.4617-1-hdanton@sina.com>
Precedence: bulk

Hi Hillf,

On Sat, Jan 07, 2023 at 08:15:29AM +0800, Hillf Danton wrote:
> On 6 Jan 2023 15:16:23 -0300 Marcelo Tosatti <mtosatti@redhat.com>
> > On Fri, Jan 06, 2023 at 11:01:54PM +0800, Hillf Danton wrote:
> > > On 6 Jan 2023 09:51:00 -0300 Marcelo Tosatti <mtosatti@redhat.com>
> > > > On Fri, Jan 06, 2023 at 08:12:44AM +0800, Hillf Danton wrote:
> > > > > 
> > > > > Regression wrt V12 if timer is added on the CPU that is not doing HK_TYPE_TIMER?
> > > > 
> > > > Before this change, the timer was managed (and queued on an isolated
> > > > CPU) by vmstat_shepherd. Now it is managed (and queued) by the local
> > > > CPU, so there is no regression.
> > > 
> > > Given vm stats folded when returning to userspace, queuing the delayed work
> > > barely makes sense in the first place. If it can be canceled, queuing it burns
> > > cycles with nothing earned. Otherwise vm stats got folded already.
> > 
> > Agree, but you can't know whether return to userspace will occur 
> > before the timer is fired.
> 
> No way to predict a random timer expiration, no?

Right.

> > 
> > So queueing the timer is to _ensure_ that eventually vmstats will be 
> > synced (which maintains the current timing behaviour wrt vmstat syncs).
> 
> After this change,
> 
> > > > > > @@ -1988,13 +2022,19 @@ void quiet_vmstat(void)
> > > > > >  	if (!is_vmstat_dirty())
> > > > > >  		return;
> > > > > >  
> 
> it is only ensured eventually by this check instead.

Yes, but if you do not return to userspace, then the per-CPU vm
statistics can be dirty indefinitely.

> > > > > > +	refresh_cpu_vm_stats(false);
> > > > > > +
> > > > > > +	if (!IS_ENABLED(CONFIG_FLUSH_WORK_ON_RESUME_USER))
> > > > > > +		return;
> > > > > > +
> > > > > > +	if (!user)
> > > > > > +		return;
> 
> 
> > Also don't think the queueing cost is significant: it only happens
> > for the first vmstat dirty item.
> 
> Cost is considered only if it is needed.

Not sure i understand what you mean (or whether there is any alternative
to the timer).