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:   Thu, 29 Jun 2023 12:09:54 +0200
From:   Frederic Weisbecker <frederic@kernel.org>
To:     Cruz Zhao <CruzZhao@linux.alibaba.com>
Cc:     gregkh@linuxfoundation.org, jirislaby@kernel.org, mingo@redhat.com,
        peterz@infradead.org, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
        bristot@redhat.com, vschneid@redhat.com, paulmck@kernel.org,
        quic_neeraju@quicinc.com, joel@joelfernandes.org,
        josh@joshtriplett.org, boqun.feng@gmail.com,
        mathieu.desnoyers@efficios.com, jiangshanlai@gmail.com,
        qiang1.zhang@intel.com, jstultz@google.com,
        clingutla@codeaurora.org, nsaenzju@redhat.com, tglx@linutronix.de,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] sched/core: introduce sched_core_idle_cpu()
Message-ID: <ZJ1YcgcuYCzwz6jI@lothringen>
References: <1688011324-42406-1-git-send-email-CruzZhao@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1688011324-42406-1-git-send-email-CruzZhao@linux.alibaba.com>
Precedence: bulk

On Thu, Jun 29, 2023 at 12:02:04PM +0800, Cruz Zhao wrote:
> As core scheduling introduced, a new state of idle is defined as
> force idle, running idle task but nr_running greater than zero.
> 
> If a cpu is in force idle state, idle_cpu() will return zero. This
> result makes sense in some scenarios, e.g., load balance,
> showacpu when dumping, and judge the RCU boost kthread is starving.
> 
> But this will cause error in other scenarios, e.g., tick_irq_exit():
> When force idle, rq->curr == rq->idle but rq->nr_running > 0, results
> that idle_cpu() returns 0. In function tick_irq_exit(), if idle_cpu()
> is 0, tick_nohz_irq_exit() will not be called, and ts->idle_active will
> not become 1, which became 0 in tick_nohz_irq_enter().
> ts->idle_sleeptime won't update in function update_ts_time_stats(), if
> ts->idle_active is 0, which should be 1. And this bug will result that
> ts->idle_sleeptime is less than the actual value, and finally will
> result that the idle time in /proc/stat is less than the actual value.
> 
> To solve this problem, we introduce sched_core_idle_cpu(), which
> returns 1 when force idle. We audit all users of idle_cpu(), and
> change idle_cpu() into sched_core_idle_cpu() in function
> tick_irq_exit().
> 
> v2-->v3: Only replace idle_cpu() with sched_core_idle_cpu() in
> function tick_irq_exit(). And modify the corresponding commit log.
> 
> Signed-off-by: Cruz Zhao <CruzZhao@linux.alibaba.com>
> Reviewed-by: Peter Zijlstra <peterz@infradead.org>
> Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
> Reviewed-by: Joel Fernandes <joel@joelfernandes.org>

Please wait for people to actually provide you with their Reviewed-by: tags
before writing it.

Aside of that, the patch looks good so you can put this one:

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>