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;
From:   "Zhang, Qiang1" <qiang1.zhang@intel.com>
To:     "paulmck@kernel.org" <paulmck@kernel.org>,
        Uladzislau Rezki <urezki@gmail.com>
CC:     "frederic@kernel.org" <frederic@kernel.org>,
        "joel@joelfernandes.org" <joel@joelfernandes.org>,
        "qiang.zhang1211@gmail.com" <qiang.zhang1211@gmail.com>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] rcu/kvfree: Prevents cache growing when the
 backoff_page_cache_fill is set
Thread-Topic: [PATCH v2] rcu/kvfree: Prevents cache growing when the
 backoff_page_cache_fill is set
Thread-Index: AQHZaBxqLyldFWH+MUeQ1f1h2hb9AK8dsuCAgADdb4CAAFfuMA==
Date:   Thu, 6 Apr 2023 23:11:37 +0000
Message-ID: <PH0PR11MB58809B44F396B7C074BDC704DA919@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20230406001238.647536-1-qiang1.zhang@intel.com>
 <ZC5MoREk30MIt79u@pc636>
 <8da4b48a-820c-461f-9dc0-a5afaf335177@paulmck-laptop>
In-Reply-To: <8da4b48a-820c-461f-9dc0-a5afaf335177@paulmck-laptop>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?8GNqW6lTCu2U9oOybpv2AiG3sYUP0MKYXZY+3tZOOc//ei/Jgy2nFIpgBH4C?=
 =?us-ascii?Q?Z0InlW0ijPb7j1kqEU5Va1WY32a3nvIqXPrYfE0raxZ8p3UF7aVUglA/2w+8?=
 =?us-ascii?Q?Znj0DKwsiC7vSJCVkQ5mtXSirFT2vOmr//KeyO2w507FgNt8Tf+jZPNtiQc7?=
 =?us-ascii?Q?Jk30n4Z6YpTMy39dCiF4myKLHafyhMCfZ1yoMqipBWrUmjpymS6mW4Smli+j?=
 =?us-ascii?Q?KZM30tGaslEngYreLAnwAEFA2L0OkBLMWQCaDnG3HxGDc22u7GbyL1pWg0CU?=
 =?us-ascii?Q?s3qEKnWZwl1bbjD+bM40FW8XBxerxka+enxa87wIdrS4girJyRa9kwLWJHZo?=
 =?us-ascii?Q?G15nV/U3pqZpPFDVmblktFtsBIrU2O2Q/mzbS3NYHEd0S2GI6x7kKQVwB6iX?=
 =?us-ascii?Q?PHTjn9XOKX6pNTuBbLQXLbuQxHuKfHplGJk0XpqNqM9TfUzO7nCCm6WvvZ6q?=
 =?us-ascii?Q?AnRqn/MMb6uRQdPBHJwzQxiGWVkGNRH8jvFZg+XDkE9i8kn8akr3TMre9kx6?=
 =?us-ascii?Q?Em0CBsphO73LyE2RuGG3NPFHwx8kc1yLr1xp+AE/YwhierTjOpr72gPPTvk+?=
 =?us-ascii?Q?OwdbulZi/WQgdV//CtVAmo3ybKCifOdZC4YqHZhlmcCV7nUmY66ZQp6ufGka?=
 =?us-ascii?Q?a/PsQAFal9a6efUYEyadsLLCzfJSpmpHE3e/kZfxEW7dIzbjqdvsrsai0/pk?=
 =?us-ascii?Q?EFiRTCf6AiCCkphJMu6wB6ack9Elt11VhVqcnAUIDhD/KV2hG29XVJw/rTrZ?=
 =?us-ascii?Q?P4cguL2cw5w522sh6/Sb3PVyQzhpdI15YtzJ6WzAFGm0BGfOZB1/GLUvmogc?=
 =?us-ascii?Q?34YqrH8rhmJdoNjt1Iuda/PRq/g3pVRCQledDPpIxnAHskt+zD3onulw/fh+?=
 =?us-ascii?Q?0zLoMtiMEM6mAMHm4UYQ76fvNgHHmcpD+TOIzIpDqArTahU2kx14Ir0pUeZ5?=
 =?us-ascii?Q?5KlVNZr595XAhTz7yk8MrdiImFFoYmQ6Mo6Ss4GOFUNPHdARsuNQdXRHCkr2?=
 =?us-ascii?Q?t8bxw3SdSh40QpqS7Jde0o8tLPYSI1eo/FLbaSwgP4WWhq1lmc7KErAqoTcX?=
 =?us-ascii?Q?YNEBbbrY60NUqp30qwrmkOqEpEM2vlGIkp4siIwG2O5qW5vX7Zad6IWN4I7W?=
 =?us-ascii?Q?f4dP93TNyA2NOn9NmaRA/ascxKJ7u/+dYKBIx2hoZZewPpMTCFFsvuJ73/jx?=
 =?us-ascii?Q?lhbVQzj6UBQKg88kl+wwpYeabZOuHzNIXe5QDQAVu7iujFoxy2HDvFfeHJkG?=
 =?us-ascii?Q?YpqY9DqRfTnOUEX+0GyfTjj4IrUx7XVM5AtdfKZuLeYAyAlX3qONexZgCnSq?=
 =?us-ascii?Q?w3cGlRQPgfFqkqASRTGwfGznIRIRUDv7SMc+lFywLfq365G0iXddxaoG/LAA?=
 =?us-ascii?Q?c34utIkwMcjhXZXy4EZiwWyYgqCH/DOon4RPXklFmNAauxeJnwNTg2GLd/6F?=
 =?us-ascii?Q?67vdZrifsVjD22a5IaIkDegNU8qJuPkv70jZTb+Yss0NroRwoQaEcxktQlm3?=
 =?us-ascii?Q?K5DEAhTtgvn4GKnGgw86s16FTUHtWOCOVFXCbL34zFaQSNatqRNxYuGn8ZfI?=
 =?us-ascii?Q?6OTboOfvz2DUNkGUBcjHbd03RGraFe4yDK5Ru2X2?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5880.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5803783b-6c1c-4f79-393c-08db36f445da
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Apr 2023 23:11:37.4899
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: K3V/vv/VYsjjdwIiwDEDSfUXjfCyzRguMMmxn+sXiZihjtSVvmcmITDgk/DKK62xDH6C61QlSUPDDl8YW0ZRvQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB7757
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,
        SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>>On Thu, Apr 06, 2023 at 06:37:53AM +0200, Uladzislau Rezki wrote:
> On Thu, Apr 06, 2023 at 08:12:38AM +0800, Zqiang wrote:
> > Currently, in kfree_rcu_shrink_scan(), the drain_page_cache() is
> > executed before kfree_rcu_monitor() to drain page cache, if the bnode
> > structure's->gp_snap has done, the kvfree_rcu_bulk() will fill the
> > page cache again in kfree_rcu_monitor(), this commit add a check
> > for krcp structure's->backoff_page_cache_fill in put_cached_bnode(),
> > if the krcp structure's->backoff_page_cache_fill is set, prevent page
> > cache growing.
> >=20
> > Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> > ---
> >  kernel/rcu/tree.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >=20
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 9cc0a7766fd2..f25430ae1936 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2907,6 +2907,8 @@ static inline bool
> >  put_cached_bnode(struct kfree_rcu_cpu *krcp,
> >  	struct kvfree_rcu_bulk_data *bnode)
> >  {
> > +	if (atomic_read(&krcp->backoff_page_cache_fill))
> > +		return false;
> >  	// Check the limit.
> >  	if (krcp->nr_bkv_objs >=3D rcu_min_cached_objs)
> >  		return false;
> > --=20
> > 2.32.0
> >
> Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
>
>Thank you both!
>
>One question, though.  Might it be better to instead modify the "for"
>loop in fill_page_cache_func() to start at krcp->nr_bkv_objs instead
>of starting at zero?  That way, we still provide a single page under
>low-memory conditions, but provide rcu_min_cached_objs of them if memory
>is plentiful.
>
>Alternatively, if we really don't want to allow any pages at all under
>low-memory conditions, shouldn't the fill_page_cache_func() set nr_pages
>to zero (instead of the current 1) when the krcp->backoff_page_cache_fill
>flag is set? =20

Hi, Paul

If the krcp->backoff_page_cache_fill is true, the put_cached_bnode () retur=
n false,
the allocated single page will also be freed in fill_page_cache_func().

or it would be better not to allocate under memory pressure.

How about like this?

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 9cc0a7766fd2..94aedbc3da36 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2907,6 +2907,8 @@ static inline bool
 put_cached_bnode(struct kfree_rcu_cpu *krcp,
        struct kvfree_rcu_bulk_data *bnode)
 {
+       if (atomic_read(&krcp->backoff_page_cache_fill))
+               return false;
        // Check the limit.
        if (krcp->nr_bkv_objs >=3D rcu_min_cached_objs)
                return false;
@@ -3220,7 +3222,7 @@ static void fill_page_cache_func(struct work_struct *=
work)
        int i;

        nr_pages =3D atomic_read(&krcp->backoff_page_cache_fill) ?
-               1 : rcu_min_cached_objs;
+               0 : rcu_min_cached_objs;

        for (i =3D 0; i < nr_pages; i++) {
                bnode =3D (struct kvfree_rcu_bulk_data *)

Thanks
Zqiang

>This would likely mean also breaking out of that loop if
>krcp->backoff_page_cache_fill was set in the meantime (which happens
>implicitly with Zqiang's patch).
>
>Or am I missing something subtle here?
>
>							Thanx, Paul