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:   Sat, 29 Jan 2022 08:37:21 -0800
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     "Zhang, Qiang1" <qiang1.zhang@intel.com>
Cc:     "frederic@kernel.org" <frederic@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] rcu: When rcuog kthreads is in polling mode, wakeup
 waitqueue is not requried
Message-ID: <20220129163721.GQ4285@paulmck-ThinkPad-P17-Gen-1>
Reply-To: paulmck@kernel.org
References: <20220128031346.157961-1-qiang1.zhang@intel.com>
 <20220129051748.GP4285@paulmck-ThinkPad-P17-Gen-1>
 <PH0PR11MB5880073B7080265117EE2868DA239@PH0PR11MB5880.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <PH0PR11MB5880073B7080265117EE2868DA239@PH0PR11MB5880.namprd11.prod.outlook.com>
Precedence: bulk

On Sat, Jan 29, 2022 at 05:55:34AM +0000, Zhang, Qiang1 wrote:
> 
> On Fri, Jan 28, 2022 at 11:13:46AM +0800, Zqiang wrote:
> > When grace period cleanup, the rcuog kthreads that waiting in sq 
> > waitqueue will be awakened, however if the 'rcu_nocb_poll' is set, the 
> > sq waitqueue always empty, so if 'rcu_nocb_poll' is set, return 
> > directly.
> 
> >This does decrease grace-period-cleanup overhead in kernels built with CONFIG_RCU_NOCB_CPU=y and booted with the rcu_nocb_poll kernel boot parameter set.  On the other hand, it increases grace-period-cleanup overhead in kernels built with CONFIG_RCU_NOCB_CPU=y but booted without the rcu_nocb_poll kernel boot parameter set.
> >
> >Last I checked, more kernels were booted without the rcu_nocb_poll kernel boot parameter set.  If this is still the case, this patch would slow things down for most systems.
> >
> >Or are there now lots of systems booted with rcu_nocb_poll?
> 
> Hi Paul
> I found that some of our customers configured rcu_nocb_poll startup parameters under Preempt-RT kernel.
> at each grace period cleanup, we'll all wakeup sq waitqueue, however when rcuog kthreads is in polling mode, 
> the wakeup operation doesn't required, because the sq waitqueue always empty.

OK, fair enough.  But was there any difference in performance measurable
at the system level?  Let's take a look at swake_up_all():

	void swake_up_all(struct swait_queue_head *q)
	{
		struct swait_queue *curr;
		LIST_HEAD(tmp);

		raw_spin_lock_irq(&q->lock);
		list_splice_init(&q->task_list, &tmp);
		while (!list_empty(&tmp)) {
			curr = list_first_entry(&tmp, typeof(*curr), task_list);

			wake_up_state(curr->task, TASK_NORMAL);
			list_del_init(&curr->task_list);

			if (list_empty(&tmp))
				break;

			raw_spin_unlock_irq(&q->lock);
			raw_spin_lock_irq(&q->lock);
		}
		raw_spin_unlock_irq(&q->lock);
	}

If the list is empty, we acquire an uncontended lock, splice an empty
list, check a pair of pointers for equality, and release that lock.
We do this once per 16 CPUs per grace period, which normally will be
every few milliseconds or less frequently.

What is the system-level performance difference and how did you measure
it?

Please don't get me wrong.  If this really is causing your users trouble,
we clearly do need to fix it.  But if so, let's do so in a way that
doesn't risk penalizing the many users who do not set rcu_nocb_poll.

							Thanx, Paul

> Thanks,
> Zqiang
> 
> >
> >							Thanx, Paul
> 
> > Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> > ---
> >  kernel/rcu/tree_nocb.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h index 
> > 636d0546a4e9..9e106c590e56 100644
> > --- a/kernel/rcu/tree_nocb.h
> > +++ b/kernel/rcu/tree_nocb.h
> > @@ -201,6 +201,8 @@ static void rcu_lockdep_assert_cblist_protected(struct rcu_data *rdp)
> >   */
> >  static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)  {
> > +	if (rcu_nocb_poll)
> > +		return;
> >  	swake_up_all(sq);
> >  }
> >  
> > --
> > 2.25.1
> >