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:     Zhen Lei <thunder.leizhen@huawei.com>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        Frederic Weisbecker <frederic@kernel.org>,
        "Neeraj Upadhyay" <quic_neeraju@quicinc.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] rcu: Dump memory object info if callback function is
 invalid
Thread-Topic: [PATCH v2] rcu: Dump memory object info if callback function is
 invalid
Thread-Index: AQHY9bjOyeVrXHxgy0el1PqNXBMQja45mfHQ
Date:   Fri, 11 Nov 2022 11:54:22 +0000
Message-ID: <PH0PR11MB5880E3A11437CD0402941F4DDA009@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20221111102822.224-1-thunder.leizhen@huawei.com>
In-Reply-To: <20221111102822.224-1-thunder.leizhen@huawei.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.500.17
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?WHC+BIqo0lHsb04lQyDhoH97Um5+ClOc9cjuZYxmJ4kgpAv6koTHyXXsWWf5?=
 =?us-ascii?Q?1SSk+NT+bL4YVrmzf7oARk16JsIvJq/6VFTaD1lv7OJcBd4bFviEBO06Axjd?=
 =?us-ascii?Q?E0gbKGfmawc7v9JPKWfYs6Bzfs/tiF5G44l9jcKKW7/RYJJNYHrEJJGKv9/F?=
 =?us-ascii?Q?+mraYOFTdJvlTRibdXwG+0s5e4t794d9GS2s9/RgXkVAeJYTCi521MYMESwM?=
 =?us-ascii?Q?6p+JyVCXLbiiBGO4QytCLUP/EEb4oExWRujuI+zzxitYQWlYOxwZwh2YJcgs?=
 =?us-ascii?Q?erLbl9aE82Uomi2bkqm10Y0EJMllTewJG5oN5bvsRm3Y6ADqqgteddDdczOg?=
 =?us-ascii?Q?gDIZvwRLZ53cD+MyiH316ZKrh8NSWAasywONcXyjhAA6/PYQppuGrayKBl6V?=
 =?us-ascii?Q?R7Pm+bdLDKIWTMD//vB7a5Uu3vNz0cUnlHumD9NKe1ShIfIxD1R+yXNsQcSW?=
 =?us-ascii?Q?JaC9tLUaigNeZLRPrNTL5d1KKkzNPMHEGMZIB6x9a74Fepsf3TypL1L1N/nJ?=
 =?us-ascii?Q?65SOMhlgUDPsAfEMMuB9/YrrI+4Ie3umZDrWiv1O/FGHX+dLcplA18N/CyYk?=
 =?us-ascii?Q?TinlqxGAoNGsGd7BCOjKONfF7JKmbiKibvKWCCJuZajq1dU07mKUpgFcNxQs?=
 =?us-ascii?Q?m8jVfxpzgeZo6OFEUFgL3AyS3PAQ4U6BZnr5ljm/gHcCrwCogVjkHWD7wu1A?=
 =?us-ascii?Q?smHq6gpMkN3NzXghOrEr+hCVCbLs1R4+qjnOZJsJsSBxKj/kkHDTQkqox9wl?=
 =?us-ascii?Q?WiJFrp0m15UjVgAOAY/S9rUSYBuo3L3V+tqewXBZKLnog88OYXhwIXv5+IU8?=
 =?us-ascii?Q?i0DP604NpDYR7L3RiaLxj1KywbJiueqfllFI4iClX1ZtGYsdtMCWzx0EqwPI?=
 =?us-ascii?Q?SdXjWYnV/l5Pqq9wQ6sClPnrtQ3DqZclPZkM+f+0yiqCH+KEc9mRT6BDOVkx?=
 =?us-ascii?Q?0f4/LO7JuH+1LufEdkH3LcgZ0/suwIeH2iKDFyoPaWFcu3yQeHoNhgpHJOej?=
 =?us-ascii?Q?XyiAS2KV7pRLHFNMg/wFm6BfEvn2lBpysONanxBpk9aNuF4dQjkG58I/0b5o?=
 =?us-ascii?Q?scWTpghSJZqncgUkCyt4M7ET0jnVIKOWT9G2t9qoJKP6nY7dP5l2TL6Ew+BX?=
 =?us-ascii?Q?JpyC1ynun63tnnXyNrKD3gbv7WWwiz9BMMzTGupK1y7CCvFah9b8nW6XAKjg?=
 =?us-ascii?Q?wt4HlsxOqUENSJFsPCEFnmdnL3P3egbT8mFvW1Crq/wUk0MRV5MPuAj038HR?=
 =?us-ascii?Q?O9SzGalttwnmPOTX8/J2wcdnXsMvQohVgysWPLXssMn2eJbqrPMCha7MhuWz?=
 =?us-ascii?Q?CkUV/039k/kf1N08til9zQM4wXknkzLc0oLT7q4TWIkZP4yJ4Im/QNb3SYLF?=
 =?us-ascii?Q?/exR3NJ/49jkl85L/IxxLMVOa2b1y+Q9yeLrnLpXkTIJN2/FNHV/zfueKIpx?=
 =?us-ascii?Q?/t6T5gLmOh7o4gyGOiwFd8FgSmCq04dm3maweKIZ+XVBUexFC/D1ToFv56aS?=
 =?us-ascii?Q?fRKBJOSMwVxKCUKvH7SwlyGsfgZtPt6UKwdB+vk9o506JdxFdVSnQQDZRJ6O?=
 =?us-ascii?Q?KtB3M0AhP5yh/NJuI8MUyCUXCbTzL5guBS/zxEnm?=
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: b6d9cdfd-fc26-4750-8bd6-08dac3db7948
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Nov 2022 11:54:22.5772
 (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: tAwiyiGTgeIdz9dFQ9Tr866/PKUuEPCf0xEVL1B72otN806odi8vJvgS1Ri2EwcV3P8yJE3fpaG1Gh+y1ehZbg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5163
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham 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

>When a structure containing an RCU callback rhp is (incorrectly) freed
>and reallocated after rhp is passed to call_rcu(), it is not unusual for
>rhp->func to be set to NULL. This defeats the debugging prints used by
>__call_rcu_common() in kernels built with CONFIG_DEBUG_OBJECTS_RCU_HEAD=3D=
y,
>which expect to identify the offending code using the identity of this
>function.
>
>And in kernels build without CONFIG_DEBUG_OBJECTS_RCU_HEAD=3Dy, things
>are even worse, as can be seen from this splat:
>
>Unable to handle kernel NULL pointer dereference at virtual address 0
>... ...
>PC is at 0x0
>LR is at rcu_do_batch+0x1c0/0x3b8
>... ...
> (rcu_do_batch) from (rcu_core+0x1d4/0x284)
> (rcu_core) from (__do_softirq+0x24c/0x344)
> (__do_softirq) from (__irq_exit_rcu+0x64/0x108)
> (__irq_exit_rcu) from (irq_exit+0x8/0x10)
> (irq_exit) from (__handle_domain_irq+0x74/0x9c)
> (__handle_domain_irq) from (gic_handle_irq+0x8c/0x98)
> (gic_handle_irq) from (__irq_svc+0x5c/0x94)
> (__irq_svc) from (arch_cpu_idle+0x20/0x3c)
> (arch_cpu_idle) from (default_idle_call+0x4c/0x78)
> (default_idle_call) from (do_idle+0xf8/0x150)
> (do_idle) from (cpu_startup_entry+0x18/0x20)
> (cpu_startup_entry) from (0xc01530)
>
>This commit therefore adds calls to mem_dump_obj(rhp) to output some
>information, for example:
>
>  slab kmalloc-256 start ffff410c45019900 pointer offset 0 size 256
>
>This provides the rough size of the memory block and the offset of the
>rcu_head structure, which as least provides at least a few clues to help
>locate the problem. If the problem is reproducible, additional slab
>debugging can be enabled, for example, CONFIG_DEBUG_SLAB=3Dy, which can
>provide significantly more information.
>
>Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>---
> kernel/rcu/rcu.h      | 7 +++++++
> kernel/rcu/srcutiny.c | 1 +
> kernel/rcu/srcutree.c | 1 +
> kernel/rcu/tasks.h    | 1 +
> kernel/rcu/tiny.c     | 1 +
> kernel/rcu/tree.c     | 1 +
> 6 files changed, 12 insertions(+)
>
>v1 --> v2:
>1. Remove condition "(unsigned long)rhp->func & 0x3", it have problems on =
x86.
>2. Paul E. McKenney helped me update the commit message, thanks.
>

Hi, Zhen Lei

Maybe the following scenarios should be considered:

                CPU 0
tasks context
   spin_lock(&vmap_area_lock)
          Interrupt=20
	 RCU softirq
	      rcu_do_batch
		mem_dump_obj
                                  vmalloc_dump_obj
                                       spin_lock(&vmap_area_lock)   <--  de=
adlock    =20

Thanks
Zqiang


>diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
>index 65704cbc9df7b3d..32ab45fabf8eebf 100644
>--- a/kernel/rcu/rcu.h
>+++ b/kernel/rcu/rcu.h
>@@ -10,6 +10,7 @@
> #ifndef __LINUX_RCU_H
> #define __LINUX_RCU_H
>=20
>+#include <linux/mm.h>
> #include <trace/events/rcu.h>
>=20
> /*
>@@ -211,6 +212,12 @@ static inline void debug_rcu_head_unqueue(struct rcu_=
head *head)
> }
> #endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>=20
>+static inline void debug_rcu_head_callback(struct rcu_head *rhp)
>+{
>+	if (unlikely(!rhp->func))
>+		mem_dump_obj(rhp);
>+}
>+
> extern int rcu_cpu_stall_suppress_at_boot;
>=20
> static inline bool rcu_stall_is_suppressed_at_boot(void)
>diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
>index 33adafdad261389..5e7f336baa06ae0 100644
>--- a/kernel/rcu/srcutiny.c
>+++ b/kernel/rcu/srcutiny.c
>@@ -138,6 +138,7 @@ void srcu_drive_gp(struct work_struct *wp)
> 	while (lh) {
> 		rhp =3D lh;
> 		lh =3D lh->next;
>+		debug_rcu_head_callback(rhp);
> 		local_bh_disable();
> 		rhp->func(rhp);
> 		local_bh_enable();
>diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
>index ca4b5dcec675bac..294972e66b31863 100644
>--- a/kernel/rcu/srcutree.c
>+++ b/kernel/rcu/srcutree.c
>@@ -1631,6 +1631,7 @@ static void srcu_invoke_callbacks(struct work_struct=
 *work)
> 	rhp =3D rcu_cblist_dequeue(&ready_cbs);
> 	for (; rhp !=3D NULL; rhp =3D rcu_cblist_dequeue(&ready_cbs)) {
> 		debug_rcu_head_unqueue(rhp);
>+		debug_rcu_head_callback(rhp);
> 		local_bh_disable();
> 		rhp->func(rhp);
> 		local_bh_enable();
>diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
>index b0b885e071fa8dc..b7f8c67c586cdc4 100644
>--- a/kernel/rcu/tasks.h
>+++ b/kernel/rcu/tasks.h
>@@ -478,6 +478,7 @@ static void rcu_tasks_invoke_cbs(struct rcu_tasks *rtp=
, struct rcu_tasks_percpu
> 	raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
> 	len =3D rcl.len;
> 	for (rhp =3D rcu_cblist_dequeue(&rcl); rhp; rhp =3D rcu_cblist_dequeue(&=
rcl)) {
>+		debug_rcu_head_callback(rhp);
> 		local_bh_disable();
> 		rhp->func(rhp);
> 		local_bh_enable();
>diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
>index bb8f7d270f01747..56e9a5d91d97ec5 100644
>--- a/kernel/rcu/tiny.c
>+++ b/kernel/rcu/tiny.c
>@@ -97,6 +97,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *hea=
d)
>=20
> 	trace_rcu_invoke_callback("", head);
> 	f =3D head->func;
>+	debug_rcu_head_callback(head);
> 	WRITE_ONCE(head->func, (rcu_callback_t)0L);
> 	f(head);
> 	rcu_lock_release(&rcu_callback_map);
>diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>index 15aaff3203bf2d0..ed93ddb8203d42c 100644
>--- a/kernel/rcu/tree.c
>+++ b/kernel/rcu/tree.c
>@@ -2088,6 +2088,7 @@ static void rcu_do_batch(struct rcu_data *rdp)
> 		trace_rcu_invoke_callback(rcu_state.name, rhp);
>=20
> 		f =3D rhp->func;
>+		debug_rcu_head_callback(rhp);
> 		WRITE_ONCE(rhp->func, (rcu_callback_t)0L);
> 		f(rhp);
>=20
>--=20
>2.25.1