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:   Sat, 21 Jan 2023 00:04:51 +0100
From:   Frederic Weisbecker <frederic@kernel.org>
To:     Joel Fernandes <joel@joelfernandes.org>
Cc:     "Zhang, Qiang1" <qiang1.zhang@intel.com>,
        "Paul E. McKenney" <paulmck@kernel.org>, quic_neeraju@quicinc.com,
        rcu@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] rcu: Remove impossible wakeup rcu GP kthread action
 from rcu_report_qs_rdp()
Message-ID: <Y8seE8Tr1w0UWf+m@lothringen>
References: <PH0PR11MB5880A16045A842AB80A25C4BDAC59@PH0PR11MB5880.namprd11.prod.outlook.com>
 <B3E458A6-9279-4716-B242-873C77EC1E3A@joelfernandes.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <B3E458A6-9279-4716-B242-873C77EC1E3A@joelfernandes.org>
Precedence: bulk

On Fri, Jan 20, 2023 at 08:27:03AM -0500, Joel Fernandes wrote:
> 
> Sure, I know what the code currently does, I am asking why and it feels wrong.
> 
> I suggest you slightly change your approach to not assuming the code should be bonafide correct and then fixing it (which is ok once in a while), and asking higher level questions to why things are the way they are in the first place (that is just my suggestion and I am not in a place to provide advice, far from it, but I am just telling you my approach — I care more about the code than increasing my patch count :P).
> 
> If you are in an intermediate state, part way to a !nocb state — you may have
> missed a nocb-related accel and wake, correct? Why does that matter? Once we
> transition to a !nocb state, we do not do a post-qs-report accel+wake anyway
> as we clearly know from the discussion.

I'm confused. We are doing that acceleration on qs report for !nocb CPU, right?

> So why do we need to do it if we
> missed it for the intermediate stage? So, I am not fully sure yet what that
> needac is doing and why it is needed.

To summarize:

* If the CPU is NOCB, all the callbacks advance and acceleration is performed
  by the rcuo/rcuog kthreads.

* If the CPU is not NOCB, all the callbacks acceleration is performed by the
  CPU, such as in the case of rcu_report_qs_rdp().

* If the CPU is transitionning from NOCB to !NOCB or from !NOCB to NOCB, the
  kthreads may not be available to do the advance/acceleration, so we must do
  it locally. That's the needacc path.

What am I missing?

Thanks.