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;
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 v2] rcu/kfree: Do not request RCU when not needed
Date:   Thu, 17 Nov 2022 08:23:30 -0500
Message-Id: <938BD0DD-9F91-44E6-BCBA-3DA3FA6779C0@joelfernandes.org>
References: <Y3YzCi9exKhiAAd0@pc636>
Cc:     paulmck@kernel.org, linux-kernel@vger.kernel.org,
        rcu@vger.kernel.org
In-Reply-To: <Y3YzCi9exKhiAAd0@pc636>
To:     Uladzislau Rezki <urezki@gmail.com>
Precedence: bulk


> On Nov 17, 2022, at 8:11 AM, Uladzislau Rezki <urezki@gmail.com> wrote:
>=20
> =EF=BB=BFOn Thu, Nov 17, 2022 at 08:06:21AM -0500, Joel Fernandes wrote:
>>=20
>>=20
>>>> On Nov 17, 2022, at 7:58 AM, Uladzislau Rezki <urezki@gmail.com> wrote:=

>>>=20
>>> =EF=BB=BFOn Wed, Nov 16, 2022 at 10:05:46PM +0000, Joel Fernandes wrote:=

>>>>> On Wed, Nov 16, 2022 at 7:19 PM Uladzislau Rezki <urezki@gmail.com> wr=
ote:
>>>>>=20
>>>>> Hello, Paul, Joel.
>>>>>=20
>>>>>>>=20
>>>>>>> Yes sure, I am doing a run now with my patch. However, I have a
>>>>>>> question -- why do you feel blocking in the kworker is not an issue?=

>>>>>>> You are taking a snapshot before queuing the normal kwork and then
>>>>>>> reading the snapshot when the normal kwork runs. Considering it is a=

>>>>>>> high priority queue, the delay between when you are taking the
>>>>>>> snapshot, and reading it is likely small so there is a bigger chance=

>>>>>>> of blocking in cond_synchronize_rcu(). Did I miss something?
>>>>>>>=20
>>>>>> We can wait indeed in the reclaim worker. But the worker does not do a=
ny
>>>>>> nasty or extra work here. If there is a need we block and wait. After=
 a
>>>>>> grace period, we are awoken and proceed.
>>>>>>=20
>>>>>> Therefore i do not see the reason in handling two cases:
>>>>>>=20
>>>>>> if (gp_done)
>>>>>>   queue_work();
>>>>>> else
>>>>>>   queue_rcu_work();
>>>>>>=20
>>>>>> it is the same if we just queue the work and check on entry. The curr=
ent
>>>>>> scenario is: queue the work after a grace period. This is the differe=
nce.
>>>>>>=20
>>>>>> Right if the reclaimer was a high prio kthread a time would be shorte=
r.
>>>>>>=20
>>>>>> In your scenario the time seems even shorter(i have not checked) beca=
use
>>>>>> you update a snapshot of krcp each time a kvfree_rcu() is invoked. So=

>>>>>> basically even though you have objects whose grace period is passed y=
ou
>>>>>> do not separate it anyhow. Because you update the:
>>>>>>=20
>>>>>> krcp->gp_snap =3D get_state_synchronize_rcu();
>>>>>>=20
>>>>>> too often.
>>>>>>=20
>>>>> Once upon a time we discussed that it is worth to keep track of GP
>>>>> per-a-page in order to reduce a memory footprint. Below patch addresse=
s
>>>>> it:
>>>>=20
>>>> In the patch below, it appears you are tracking the GP per krwp, and
>>>> not per page. But I could be missing something - could you split it
>>>> into separate patches for easier review?
>>>>=20
>>> I will split. I was thinking about it. The GP is tracked per-a-page. As f=
or
>>> krwp it is only for channel_3. Everything goes there if no-page or no ca=
che.
>>>=20
>> Ah, ok.
>>=20
>>>>=20
>>>> Also it still does cond_synchronize_rcu() :-(
>>>>=20
>>> Sometimes we need to wait for a GP we can not just release :)
>>=20
>> You know that is not what I meant ;) I was concerned about the blocking.
>>=20
> Let me split. After that we/you can test and check if there is any issue
> with sleeping on entry for waiting a GP if needed.

Ack. What I=E2=80=99ll also do is, whenever you split it, I=E2=80=99ll put i=
t on ChromeOS and see in real world, how many times we block. It could be I=E2=
=80=99m missing something of how polled GP works.

thanks,

 - Joel=20

>=20
> --
> Uladzislau Rezki