Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 3 Jul 2020 11:02:26 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Mel Gorman <mgorman@techsingularity.net>
Cc:     Dave Jones <davej@codemonkey.org.uk>,
        Linux Kernel <linux-kernel@vger.kernel.org>, mingo@kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: weird loadavg on idle machine post 5.7
Message-ID: <20200703090226.GV4800@hirez.programming.kicks-ass.net>
References: <20200702171548.GA11813@codemonkey.org.uk>
 <20200702213627.GF3183@techsingularity.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200702213627.GF3183@techsingularity.net>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 02, 2020 at 10:36:27PM +0100, Mel Gorman wrote:

> > commit c6e7bd7afaeb3af55ffac122828035f1c01d1d7b (refs/bisect/bad)
> > Author: Peter Zijlstra <peterz@infradead.org>

> Peter, I'm not supremely confident about this but could it be because
> "p->sched_contributes_to_load = !!task_contributes_to_load(p)" potentially
> happens while a task is still being dequeued? In the final stages of a
> task switch we have
> 
>         prev_state = prev->state;
>         vtime_task_switch(prev);
>         perf_event_task_sched_in(prev, current);
>         finish_task(prev);
> 
> finish_task is when p->on_cpu is cleared after the state is updated.
> With the patch, we potentially update sched_contributes_to_load while
> p->state is transient so if the check below is true and ttwu_queue_wakelist
> is used then sched_contributes_to_load was based on a transient value
> and potentially wrong.

I'm not seeing it. Once a task hits schedule(), p->state doesn't change,
except through wakeup.

And while dequeue depends on p->state, it doesn't change it.

At this point in ttwu() we know p->on_rq == 0, which implies dequeue has
started, which means we've (at least) stopped executing the task -- we
started or finished schedule().

Let me stare at this more...