Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Joel Fernandes <joel@joelfernandes.org>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH v3] rcu: Remove impossible wakeup rcu GP kthread action from rcu_report_qs_rdp()
Date:   Mon, 23 Jan 2023 15:54:07 -0500
Message-Id: <375EBA2F-F47A-4BC6-B4D4-2E96D5D15AB6@joelfernandes.org>
References: <Y861Xuoa+CrZt9PB@lothringen>
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
In-Reply-To: <Y861Xuoa+CrZt9PB@lothringen>
To:     Frederic Weisbecker <frederic@kernel.org>
Precedence: bulk


> On Jan 23, 2023, at 11:27 AM, Frederic Weisbecker <frederic@kernel.org> wr=
ote:
>=20
> =EF=BB=BFOn Mon, Jan 23, 2023 at 10:22:19AM -0500, Joel Fernandes wrote:
>>> What am I missing?
>>=20
>> That the acceleration is also done by __note_gp_changes() once the
>> grace period ends anyway, so if any acceleration was missed as you
>> say, it will be done anyway.
>>=20
>> Also it is done by scheduler tick raising softirq:
>>=20
>> rcu_pending() does this:
>>        /* Has RCU gone idle with this CPU needing another grace period? *=
/
>>        if (!gp_in_progress && rcu_segcblist_is_enabled(&rdp->cblist) &&
>>            !rcu_rdp_is_offloaded(rdp) &&
>>            !rcu_segcblist_restempty(&rdp->cblist, RCU_NEXT_READY_TAIL))
>>                return 1;
>>=20
>> and rcu_core():
>>        /* No grace period and unregistered callbacks? */
>>        if (!rcu_gp_in_progress() &&
>>            rcu_segcblist_is_enabled(&rdp->cblist) && do_batch) {
>>                rcu_nocb_lock_irqsave(rdp, flags);
>>                if (!rcu_segcblist_restempty(&rdp->cblist, RCU_NEXT_READY_=
TAIL))
>>                        rcu_accelerate_cbs_unlocked(rnp, rdp);
>>                rcu_nocb_unlock_irqrestore(rdp, flags);
>>        }
>>=20
>> So, I am not sure if you need needacc at all. Those CBs that have not
>> been assigned grace period numbers will be taken care off :)
>=20
> But that's only when there is no grace period pending, so it can't happen w=
hile
> we report a QS.
>=20
> OTOH without the needacc, those callbacks waiting to be accelerated would b=
e
> eventually processed but only on the next tick following the end of a grac=
e
> period...if none has started since then. So if someone else starts a new G=
P
> before the current CPU, we must wait another GP, etc...
>=20
> That's potentially dangerous.

Waiting for just one more GP cannot be dangerous IMO. Anyway there is no gua=
rantee that callback will run immediately at end of GP, there may be one or m=
ore GPs before callback can run, if I remember correctly. That is by design.=
. but please correct me if my understanding is different from yours.

>=20
> And unfortunately we can't do the acceleration from __note_gp_changes() du=
e
> to lock ordering restrictions: nocb_lock -> rnp_lock
>=20

Ah. This part I am not sure. Appreciate if point me to any old archive links=
 or documentation detailing that, if possible=E2=80=A6=20

Thanks!

- Joel

>>=20
>> Thanks!
>>=20
>> -Joel