Received-SPF: pass (google.com: domain of linux-kernel+bounces-104086-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: "Zhang, Qiang4" <qiang4.zhang@intel.com>
To: Nathan Chancellor <nathan@kernel.org>
CC: Andrew Morton <akpm@linux-foundation.org>, Nick Desaulniers
	<ndesaulniers@google.com>, Bill Wendling <morbo@google.com>, Justin Stitt
	<justinstitt@google.com>, "linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"llvm@lists.linux.dev" <llvm@lists.linux.dev>, "Stable@vger.kernel.org"
	<Stable@vger.kernel.org>, "Zhang, Qiang4" <qiang4.zhang@intel.com>
Subject: RE: [PATCH] memtest: use {READ,WRITE}_ONCE in memory scanning
Thread-Topic: [PATCH] memtest: use {READ,WRITE}_ONCE in memory scanning
Thread-Index: AQHadFQSJGEbnIyyQ0G1fVC+polJ+bE17SuAgAJYgTA=
Date: Fri, 15 Mar 2024 06:24:11 +0000
Message-ID: <DM4PR11MB6020ECE5592459BB33281A16DF282@DM4PR11MB6020.namprd11.prod.outlook.com>
References: <20240312080422.691222-1-qiang4.zhang@intel.com>
 <20240313172141.GB3064248@dev-arch.thelio-3990X>
In-Reply-To: <20240313172141.GB3064248@dev-arch.thelio-3990X>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?l3YOA1FWR4LPdfXH5QCObAt0Tg1jCNIP8+ss/J9DqHzjKr0d6s81Dwe7U+5F?=
 =?us-ascii?Q?yLGR/Y995A3tj66XJLrpNRylyOJZ6Q2iYL6dXtzh38Tvs4Qm2C5Yn5BMzDk9?=
 =?us-ascii?Q?U2tSeOwyp4v1bF4HqNlwGe4uIvh3YAl4Iv6DZ8ovDvWQaAP50nyZOTbJtYmS?=
 =?us-ascii?Q?JmN0UNx7GYVElV9XSvMrIUDJLEZw+A40PygBqZfQxzoFk3GW/UdtiL/SqzDL?=
 =?us-ascii?Q?7F2B2nU9giJOlRodOXFT/gcgvlU+VJcd+q8I6fkjR1EBKxNPY6YC+cDigfgy?=
 =?us-ascii?Q?78uLO6YutldFW2x+Cg0RnM3r/MNx6HphRnJmnut1VSQOU+HSPfcQNtWVcLqt?=
 =?us-ascii?Q?WZjp0qN/38i1rZwvrc67ctQh8e4ZTNiX+pCLUVwau+HMigvuDWkUS9zzCoeI?=
 =?us-ascii?Q?88D6wY/rpeLV7OFmdG21qQki/FaYQ/3s1ClyXXxLzhOUq+Xyf9/Mv1FpyDz1?=
 =?us-ascii?Q?Oaaev3mNOdJFr5gmVBJSsvna0RPcaScAfvYATqNJoTYLJ5a6Mi0L81WUTZru?=
 =?us-ascii?Q?nVmQWyMAObhAEFppxJtUenz80uBR2QKhYtluybglyT4Ab6+MLuPyjp88dPJ+?=
 =?us-ascii?Q?RCOQE+m4WvBrIqgDsbot0ctyfG5Ts8hXLf3+RKeMOeMsOPRxZ1btNY7OMtV2?=
 =?us-ascii?Q?JLTw5uqW7U2/oS/vF3Hgz+6Tqr5ojX8cG3Ra750/LdEcXwibaJOuPDgTLUlN?=
 =?us-ascii?Q?VW1s4ZpETl7HkafTGgI3B+HK+Xguo4rKj4ecXxn9RT9e8AJ7Yh5KIXniTk1i?=
 =?us-ascii?Q?eAk1ahQNNqP7Xr007WvsPit5jmHslT7BM2AYULUMeZ/3+/sa/QSZ5fmZAUTd?=
 =?us-ascii?Q?gFm3GJEa6u8hyOZz3DWEx2t5J/xZG6FBPxfKg7KRAtBkHkpc7F8agGBAofWF?=
 =?us-ascii?Q?gicsuMxILW3e96oGzShfMadGmDTmvOFSPghAcPiMCEODSJoHNIz/DrVtyxFH?=
 =?us-ascii?Q?46UvIB1HIYl/UTpdsGrxuagcp+7WOIq7u9Y3xPclQEZ8+neXp+A0QLJKG04r?=
 =?us-ascii?Q?G259QM1RH42Cj0Lgwt6ea3dFOI3/fVTjucUklpQJ+N/gY9I01XEQSQJKPhoj?=
 =?us-ascii?Q?4fLxDfzqfYOxQImEpPM00LL9xcl+PRzOeamk004voFn9vwFN95ufipDETSwU?=
 =?us-ascii?Q?JLoU9tG6PQI62KyO0LqbuQdCDYuYBW8moPs1Kt3p64TCEYzS6e/sYXuKztXT?=
 =?us-ascii?Q?KqFj3sn4TitaCvo6at9TW/wWgfqb4ljUAz6QxfCXsiDJ0GtqFPxmMGFCnA1N?=
 =?us-ascii?Q?9hfw243+zlNkXkbqN0OxmBrzkTRfTlgtoyKVEZk5eqLjTD7bWYztFdLtNekd?=
 =?us-ascii?Q?51VvMiUjgUdlV7TYw+ld+/VBGk+2TDZEzktsyk8br3aMBLFWxCm3iHah+dkj?=
 =?us-ascii?Q?UH/eD5f61J7R55i9yLgtpmvF23QubeNKmiQaa6bRLwsQwrmsfOSJN/j3EF2B?=
 =?us-ascii?Q?+l25j92evlYFe835VbyUEN9k1FJqMVSBnpFd3h5bHqBjnKgRZPQz3p40+KCo?=
 =?us-ascii?Q?UZlUf1rkhUCEYZC+fg6Ym0+JeF+qI0j6BxUem1RQURMUjs64AvGJOg9wUb79?=
 =?us-ascii?Q?m4/GdH8pmJqrz7O0vy1w4+jIb75vnlK4QaYc9ro0?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6020.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d1c3e526-1d83-48e3-787e-08dc44b8877a
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Mar 2024 06:24:11.7091
 (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: IPiWdCoSuhCgshJ+uY60LQfZ3uOiqvmgXNifBHSwiZ8pUEu/wUeNA53u0SJg3MyaVeqeJcTC76PzPST+RlH7Nw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB7117
X-OriginatorOrg: intel.com

Hi, Nathan

Sorry for the incomplete commit message.=20

I have tried to compile with gcc and clang-{11,13,14} on Debian 12.  On my =
test environment, hypervisor emulates a range of bad memory where writes ar=
e ignored and reads always returns all ones.
Memtest compiled with all clang-{11,13,14} can't find the bad memory withou=
t this patch. But gcc works fine. So it seems not a regression in clang.
I don't have expertise in compilers. But I think {READ,WRITE}_ONCE can forc=
e the compiler to treat the iterating pointer as volatile.

Welcome more comments !=20

BR
Qiang

-----Original Message-----
From: Nathan Chancellor <nathan@kernel.org>=20
Sent: Thursday, March 14, 2024 1:22 AM
To: Zhang, Qiang4 <qiang4.zhang@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>; Nick Desaulniers <ndesaulnie=
rs@google.com>; Bill Wendling <morbo@google.com>; Justin Stitt <justinstitt=
@google.com>; linux-mm@kvack.org; linux-kernel@vger.kernel.org; llvm@lists.=
linux.dev; Stable@vger.kernel.org
Subject: Re: [PATCH] memtest: use {READ,WRITE}_ONCE in memory scanning

On Tue, Mar 12, 2024 at 04:04:23PM +0800, Qiang Zhang wrote:
> memtest failed to find bad memory when compiled with clang. So use=20
> {WRITE,READ}_ONCE  to access memory to avoid compiler over optimization.

This commit message is severely lacking in details in my opinion, especiall=
y for a patch marked for stable. Did a kernel or LLVM change cause this (i.=
e., has this always been an issue or is it a recent regression)? What is th=
e transformation that LLVM does to break the test and why is using READ_ONC=
E() or WRITE_ONCE() sufficient to resolve it?

> Cc: <Stable@vger.kernel.org>
> Signed-off-by: Qiang Zhang <qiang4.zhang@intel.com>
> ---
>  mm/memtest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>=20
> diff --git a/mm/memtest.c b/mm/memtest.c index=20
> 32f3e9dda837..c2c609c39119 100644
> --- a/mm/memtest.c
> +++ b/mm/memtest.c
> @@ -51,10 +51,10 @@ static void __init memtest(u64 pattern, phys_addr_t s=
tart_phys, phys_addr_t size
>  	last_bad =3D 0;
> =20
>  	for (p =3D start; p < end; p++)
> -		*p =3D pattern;
> +		WRITE_ONCE(*p, pattern);
> =20
>  	for (p =3D start; p < end; p++, start_phys_aligned +=3D incr) {
> -		if (*p =3D=3D pattern)
> +		if (READ_ONCE(*p) =3D=3D pattern)
>  			continue;
>  		if (start_phys_aligned =3D=3D last_bad + incr) {
>  			last_bad +=3D incr;
> --
> 2.39.2
>=20