From:   "Zhang, Qiang1" <qiang1.zhang@intel.com>
To:     "paulmck@kernel.org" <paulmck@kernel.org>
CC:     "mingo@redhat.com" <mingo@redhat.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "frederic@kernel.org" <frederic@kernel.org>,
        "joel@joelfernandes.org" <joel@joelfernandes.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] sched/isolation: Fix illegal CPU value by
 housekeeping_any_cpu() return
Thread-Topic: [PATCH] sched/isolation: Fix illegal CPU value by
 housekeeping_any_cpu() return
Thread-Index: AQHZPJqZxYoAdHprS0OYkz1Mn3AM+a7G63MAgABd5FA=
Date:   Thu, 9 Feb 2023 23:52:06 +0000
Message-ID: <PH0PR11MB5880CBA66474C7BA0FBE06C5DAD99@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20230209144911.987938-1-qiang1.zhang@intel.com>
 <20230209181420.GM2948950@paulmck-ThinkPad-P17-Gen-1>
In-Reply-To: <20230209181420.GM2948950@paulmck-ThinkPad-P17-Gen-1>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?gYF1/dC8kx+WwC24qDM3+TLLd3Aznqgobn+JkiBSjcSRHSXVQZBT6DLvXaFv?=
 =?us-ascii?Q?EZU8Ud7Y2C6MDNe+e6mqeqWRBfwuJZ8u+9f0826mdyFWDfLv/U7yaKzeV+1/?=
 =?us-ascii?Q?93oV7qqmwSDY2zVXMU7tdhH21smrXRO1YALcJl8Oyi/dAnnjxShSZZcGL7wv?=
 =?us-ascii?Q?ntzkNpCpiTjjzsx5dg9/iSODDA8za+t65pkuqDRx74pVmHIuHA7JDWsdTk18?=
 =?us-ascii?Q?htbWiiFiGoLBBrFF39EaaUy/2OcxPuqjQcip/FL0LfPpG0KeLvul1Vm9xp5X?=
 =?us-ascii?Q?UqXPkf2yIEYd664cLrxpqkhs7i6evupwu3HqxXIrvq3vmrUcEj3IrPrLI28g?=
 =?us-ascii?Q?MMzv11VlI9wELQHx0eCxPeiFyP+0c1qSdhGG/ZRfpsjvsuqIvl7fD9Tj6GrM?=
 =?us-ascii?Q?HITASmw5+xgu7FXlup4c5roo2I3eFcH9oMCCpNaUKAbqfr9DyoE73fQ8+qO2?=
 =?us-ascii?Q?xcMWNljIB09+Rob4YX8JRcz8zVJaSK9fA/CnAYs/bP4UgkKoW50bR/QMkWTA?=
 =?us-ascii?Q?T7OfITKlu5xX+aRuZuRl3ridX/l+kdT0evEoHz2BdzB1PSaVFz1rzJbXL2kM?=
 =?us-ascii?Q?73cTEQnvLfVLLGKQr+YPpHnp61I9uWPSiz8G4zdsRSNhOTuB4XC1cusygRqT?=
 =?us-ascii?Q?2uKwgdGX4bH3axGrAtyjqcW1y68DYCU+tRVw43tN2Dig+I3GEZHXgwbPk0Sa?=
 =?us-ascii?Q?uZUUiFI05Q+qo0HWXAoEiO5r5IfnvYDMqskk639zjTB1RG50cBm5E+Hg5piX?=
 =?us-ascii?Q?OHnQ5MEUCzCAuAD87JxWEB+3gvN3XctD/6+OiKkIXZs5a1DaWUzEYwb64XHW?=
 =?us-ascii?Q?Bx6zDwgZ3hqExgH5Q4eV2d55rsUZmsStKCCKVZL7ZaMNhC3ldXNtd7snMfd/?=
 =?us-ascii?Q?8hiDsoE3EhF1Hkhnj+6v+fVoiaI9zsyWSnp74CqhlWI5oD9vRBWGRirHAsK+?=
 =?us-ascii?Q?UpAl1dBr6EzJuxXYnYD7/jGaYZ0TeUd9HuqIBqNobZCCX+lNHVqTFocw+2OZ?=
 =?us-ascii?Q?+C8OgdNtN/8LUHLog4nN1WNbf9anOOruuCgnrXC0sr1vUjxGtSqLF3lSqzwh?=
 =?us-ascii?Q?U8AGLPiYlHJJdu2yh5jcv/L+9c4XFDK8M2CMMk6e30Q1UJS512MXLA+dAm6m?=
 =?us-ascii?Q?A18lJIrbGbCxQE0lAX+6/zInP8PnuNee5+ToWW6kQkNP9uZAu4HvE8f9KoYy?=
 =?us-ascii?Q?aKKeINKkLMFUh+71K9NJsNY3syaXEMKqbppkk/KCDqzpZvxpEYA5C7sm8iDe?=
 =?us-ascii?Q?d5KZp8iOWR+ShwBaeja7xLhjXtG9B7SYLfEhcYlk29A5HYqSlVpaxhIXsHhs?=
 =?us-ascii?Q?Yf+c4lucOUSVGWnrw40VHcTmWIPzYchMXhWbWNiCFDf4o3Qq+W20AV2x5UbI?=
 =?us-ascii?Q?4BgXkelqveqgy0OtLCJKaoxcj1saWFcEowg5epijsUjJlGM93XQE4t2CGd4W?=
 =?us-ascii?Q?nzxq6DkUgjCpBSFapPT4Qgf9gZ+qZxdthBRVGc40H+B3KpIVHZdJXPLoUpLm?=
 =?us-ascii?Q?/UhEXCefgkdLPCV4XsZaO3H9PfgY3OT00dM/PASbrOtJV04SdCfrMatb8rr5?=
 =?us-ascii?Q?2OX65l9UZw1Q0VuUMV2GBZEXKQ9pYK49cPb7IyyP?=
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: d043b973-6a9b-4a8a-0e7a-08db0af8a66f
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Feb 2023 23:52:06.3207
 (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: wLIAgJVF3YcFBOZcn8YjhJd6HJlAtv6O22bP/JGh0Fe46WdrjOzoYISnzfziEKTkSaL7DY655NuhfKAM3EkfJg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB7487
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On Thu, Feb 09, 2023 at 10:49:11PM +0800, Zqiang wrote:
> For kernels built with CONFIG_NO_HZ_FULL=3Dy, running the following tests=
:
>=20
> runqemu kvm slirp nographic qemuparams=3D"-m 1024 -smp 4" bootparams=3D
> "console=3DttyS0 nohz_full=3D0,1 rcu_nocbs=3D0,1 sched_verbose" -d
>=20
> root@qemux86-64:~# echo 0 > /sys/devices/system/cpu/cpu2/online
> root@qemux86-64:~# echo 0 > /sys/devices/system/cpu/cpu3/online
>=20
> [   22.838290] BUG: unable to handle page fault for address: ffffffff84cd=
48c0
> [   22.839409] #PF: supervisor read access in kernel mode
> [   22.840215] #PF: error_code(0x0000) - not-present page
> [   22.841028] PGD 3e19067 P4D 3e19067 PUD 3e1a063 PMD 800ffffffb3ff062
> [   22.841889] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN PTI
> [   22.842175] CPU: 0 PID: 16 Comm: rcu_preempt Not tainted 6.2.0-rc1-yoc=
to-standard+ #658
> [   22.842534] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
>                BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.o4
> [   22.843036] RIP: 0010:do_raw_spin_trylock+0x70/0x120
> [   22.843267] Code: 81 c7 00 f1 f1 f1 f1 c7 40 04 04 f3 f3 f3 65
>                48 8b 04 25 28 00 00 00 48 89 45 e0 31 c0 e8 b8 0
> [   22.844187] RSP: 0018:ffff8880072b7b30 EFLAGS: 00010046
> [   22.844429] RAX: 0000000000000000 RBX: ffffffff84cd48c0 RCX: dffffc000=
0000000
> [   22.844751] RDX: 0000000000000003 RSI: 0000000000000004 RDI: ffffffff8=
4cd48c0
> [   22.845074] RBP: ffff8880072b7ba8 R08: ffffffff811daa20 R09: fffffbfff=
099a919
> [   22.845400] R10: ffffffff84cd48c3 R11: fffffbfff099a918 R12: 1ffff1100=
0e56f66
> [   22.845719] R13: ffffffff84cd48d8 R14: ffffffff84cd48c0 R15: ffff88800=
72b7cd8
> [   22.846040] FS:  0000000000000000(0000) GS:ffff888035200000(0000) knlG=
S:0000000000000000
> [   22.846403] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   22.846667] CR2: ffffffff84cd48c0 CR3: 000000001036c000 CR4: 000000000=
01506f0
> [   22.846988] Call Trace:
> [   22.847105]  <TASK>
> [   22.847208]  ? __pfx_do_raw_spin_trylock+0x10/0x10
> [   22.847430]  ? rcu_read_unlock+0x26/0x80
> [   22.847612]  ? trace_preempt_off+0x2a/0x130
> [   22.847812]  _raw_spin_lock+0x41/0x80
> [   22.847984]  ? schedule_timeout+0x242/0x580
> [   22.848178]  schedule_timeout+0x242/0x580
> [   22.848366]  ? __pfx_schedule_timeout+0x10/0x10
> [   22.848575]  ? __pfx_do_raw_spin_trylock+0x10/0x10
> [   22.848796]  ? __pfx_process_timeout+0x10/0x10
> [   22.849005]  ? _raw_spin_unlock_irqrestore+0x46/0x80
> [   22.849232]  ? prepare_to_swait_event+0xb8/0x210
> [   22.849450]  rcu_gp_fqs_loop+0x66e/0xe70
> [   22.849633]  ? rcu_gp_init+0x87c/0x1130
> [   22.849813]  ? __pfx_rcu_gp_fqs_loop+0x10/0x10
> [   22.850022]  ? _raw_spin_unlock_irqrestore+0x46/0x80
> [   22.850251]  ? finish_swait+0xce/0x100
> [   22.850429]  rcu_gp_kthread+0x2ea/0x6b0
> [   22.850608]  ? __pfx_do_raw_spin_trylock+0x10/0x10
> [   22.850829]  ? __pfx_rcu_gp_kthread+0x10/0x10
> [   22.851039]  ? __kasan_check_read+0x11/0x20
> [   22.851233]  ? __kthread_parkme+0xe8/0x110
> [   22.851424]  ? __pfx_rcu_gp_kthread+0x10/0x10
> [   22.851627]  kthread+0x172/0x1a0
> [   22.851781]  ? __pfx_kthread+0x10/0x10
> [   22.851956]  ret_from_fork+0x2c/0x50
> [   22.852129]  </TASK>
>=20
> schedule_timeout()
> ->__mod_timer()
>  ->get_target_base(base, timer->flags)
>    ->get_timer_cpu_base(tflags, get_nohz_timer_target());
>      ->cpu =3D get_nohz_timer_target()
>              ->housekeeping_any_cpu(HK_TYPE_TIMER)
>                      /*housekeeping.cpumasks[type] is 2-3*/
> 		     /*cpu_online_mask is 0-1*/
>                ->cpu =3D cpumask_any_and(housekeeping.cpumasks[type],
> 			cpu_online_mask);
>              /*cpu value is 4*/
>      ->new_base =3D per_cpu_ptr(&timer_bases[BASE_DEF], cpu);
>    /*new_base is illegal address*/
>  ->if (base !=3D new_base)
>    ->raw_spin_lock(&new_base->lock); =3D=3D> trigger Oops
>=20
> This commit therefore add checks for cpumask_any_and() return values
> in housekeeping_any_cpu(), if cpumask_any_and() returns an illegal CPU
> value, the housekeeping_any_cpu() will return current CPU number.
>=20
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---
>  kernel/sched/isolation.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>=20
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 373d42c707bc..e255eb83f14f 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -46,7 +46,9 @@ int housekeeping_any_cpu(enum hk_type type)
>  			if (cpu < nr_cpu_ids)
>  				return cpu;
> =20
> -			return cpumask_any_and(housekeeping.cpumasks[type], cpu_online_mask);
> +			cpu =3D cpumask_any_and(housekeeping.cpumasks[type], cpu_online_mask)=
;
> +			if (cpu >=3D nr_cpu_ids)
> +				return smp_processor_id();
>
>
>Don't we need a "return cpu" here?  Or am I missing something subtle?

Sorry, thanks for review, I will change and resend.

Thanks
Zqiang

>
>							Thanx, Paul
>
>  		}
>  	}
>  	return smp_processor_id();
> --=20
> 2.25.1
>=20