Date:   Tue, 24 Jan 2023 17:10:21 +0000
From:   Joel Fernandes <joel@joelfernandes.org>
To:     "Zhang, Qiang1" <qiang1.zhang@intel.com>
Cc:     "Paul E. McKenney" <paulmck@kernel.org>,
        "frederic@kernel.org" <frederic@kernel.org>,
        "quic_neeraju@quicinc.com" <quic_neeraju@quicinc.com>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@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: <Y9AQ/Yob7x/dif82@google.com>
References: <PH0PR11MB5880A16045A842AB80A25C4BDAC59@PH0PR11MB5880.namprd11.prod.outlook.com>
 <B3E458A6-9279-4716-B242-873C77EC1E3A@joelfernandes.org>
 <PH0PR11MB58803DFEDD61EE66AF49074EDACA9@PH0PR11MB5880.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <PH0PR11MB58803DFEDD61EE66AF49074EDACA9@PH0PR11MB5880.namprd11.prod.outlook.com>
Precedence: bulk

On Sat, Jan 21, 2023 at 12:38:35AM +0000, Zhang, Qiang1 wrote:
[...]
> >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
> 
> Should it be transition to a !nocb state, we do a post-qs-report accel+wake.
> 
> >as we clearly know from the discussion. 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.
> 
> For de-offloading, when in the process of
> de-offloading(rcu_segcblist_completely_offloaded() return false), we're not
> doing bypass even though this rdp is offloaded
> state(rcu_rdp_is_offloaded(rdp) return true), at this time, the rcuog
> kthread probably won't accel+wakeup, so we do accel+wakeup in
> rcu_report_qs_rdp(), as you say why does that matter?  for !nocb state,
> we've always tried to accel+wakeup as much as possible(compared to nocb),
> let rcu callback be executed as soon as possible.
> 
> This is just my personal opinion, please correct me if I am wrong.

I think your opinion is correct. The acceleration after the QS reporting may
be needed for the case where we are part way between the offload and
de-offload state as in this state (which we could call limbo state), there
does not seem to be anywhere that acceleration is performed, and if this state
persists for a long period of time, then no other path can accelerate the CBs
thus likely starving them as Frederic mentioned..

thanks,

 - Joel


> Thanks
> Zqiang
> 
> 
> >
> >Do not get me wrong, stellar work here. But I suggest challenge the assumptions and the design, not always just the code that was already written :), apologies for any misplaced or noisy advice.
> >
> >Thanks!
> >
> > - Joel
> 
> 
> >    
> > Thanks
> > Zqiang
> > 
> >> 
> >> (I am out of office till Monday but will intermittently (maybe) check
> >> in, RCU is one of those things that daydreaming tends to lend itself
> >> to...)
> >> 
> >> - Joel