From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 4653244E055 for ; Wed, 8 Jul 2026 12:43:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783514619; cv=none; b=rsmxvv7YTHC6waKZmNLXy60Nwa3J9DNiKt8eopHw36Uic9jOU/NZo+RjRLaDLfYc4A/8OlvV2h/1SBaxiTCESyWY03oXA4UVbdIx34EjkBdRhPMZRGNWt5wN4tWxAI7CRSXon22UEao1o/zxnG0kONw+DbIebhREFijwWxStOtE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783514619; c=relaxed/simple; bh=gWp+3yzItFKmNeTeCbEZqOw1SYe7BR0bUIaJS8T+B6o=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=LcqaeKTF3qGTIa+Y0sQnaoJdiM/cTPP/aUbRjbb2UyiOrol3xkhV8So5xRCZn/8zEbcP6xr8O+NnrYSidQ5AWq8z5LOtO3LxR2g+YHc3yLSlli96Tf5CqXOAC5Wsd2lOeiemqyqI7gUXw1McBJFjgKsWRIhL6n4eb6PpKuWoZWU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=ulcn4CO9; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="ulcn4CO9" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DDAB82F; Wed, 8 Jul 2026 05:43:30 -0700 (PDT) Received: from [10.1.30.54] (unknown [10.1.30.54]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E0D6C3F7B4; Wed, 8 Jul 2026 05:43:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1783514615; bh=gWp+3yzItFKmNeTeCbEZqOw1SYe7BR0bUIaJS8T+B6o=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=ulcn4CO99/3f/6uXCB350dwHaGdTkecbpP535w1xCRxO5v4pJCASINSJJ6EO4iHRG QgQB0TEcCzb4MhMfw8AAguXxmbdeGdTg40kMoaWx8huTY/bQostBbsutCL51JekyW8 k3ei7oY0ouAN5Pijg9THZ0dughBzpr9NLIEoeHLg= Message-ID: <724a080a-bd4b-4fab-a890-c6a97ebff77f@arm.com> Date: Wed, 8 Jul 2026 13:43:32 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V2] mm: Standardize printing for pgtable entries Content-Language: en-GB To: "David Hildenbrand (Arm)" , Anshuman Khandual , linux-mm@kvack.org Cc: andriy.shevchenko@linux.intel.com, usama.arif@linux.dev, hughd@google.com, willy@infradead.org, Andrew Morton , linux-kernel@vger.kernel.org References: <20260708032824.969752-1-anshuman.khandual@arm.com> <25cde246-540f-4ab4-b541-0b7313c57954@arm.com> <76f24730-46de-4583-9189-ff7ba28c741d@kernel.org> From: Ryan Roberts In-Reply-To: <76f24730-46de-4583-9189-ff7ba28c741d@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 08/07/2026 12:27, David Hildenbrand (Arm) wrote: > On 7/8/26 13:22, Ryan Roberts wrote: >> On 08/07/2026 04:28, Anshuman Khandual wrote: >>> From: "David Hildenbrand (Arm)" >>> >>> Bad page map reporting currently stores page table entry values in an >>> unsigned long long and prints them with fixed 64-bit-oriented format >>> strings. This is inconsistent across call sites and does not work well for >>> architectures where page table entry values are not naturally represented >>> as 64-bit values, such as 32-bit or 128-bit entries. >>> >>> Introduce a common helper to convert raw page table entry values into a >>> fixed-width hexadecimal string based on the actual entry size. Use it for >>> bad page map reporting and for dumping the page table walk in >>> __print_bad_page_map_pgtable(). >>> >>> Pass page table entry values to the reporting path as raw bytes together >>> with their size, instead of forcing them through an unsigned long long. >>> It keeps the printed output consistent and avoids truncation or misleading >>> formatting for non-64-bit page table entries. >>> >>> Cc: Andrew Morton >>> Cc: linux-mm@kvack.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: David Hildenbrand (Arm) >>> Signed-off-by: Anshuman Khandual >>> --- >>> This patch applies on v7.2-rc2 >>> >>> Changes in V2: >>> >>> - Dropped space after ":" during print per Matthew >>> - Dropped CONFIG_CPU_BIG_ENDIAN per David >>> >>> Changes in V1: >>> >>> https://lore.kernel.org/all/20260707041703.658021-1-anshuman.khandual@arm.com/ >>> >>> mm/memory.c | 98 ++++++++++++++++++++++++++++++++++++++++------------- >>> 1 file changed, 75 insertions(+), 23 deletions(-) >>> >>> diff --git a/mm/memory.c b/mm/memory.c >>> index ff338c2abe92..a2b63af82792 100644 >>> --- a/mm/memory.c >>> +++ b/mm/memory.c >>> @@ -519,9 +519,48 @@ static bool is_bad_page_map_ratelimited(void) >>> return false; >>> } >>> >>> +#define PTVAL_STR_MAX (32 + 1) /* Max 128-bit value in hex + NUL */ >> >> Not sure if it's worth doing something like this?: >> >> #define PTVAL_STR_MAX \ >> (MAX(MAX(MAX(MAX(sizeof(pteval_t), sizeof(pmdval_t)), \ >> sizeof(pudval_t)), \ >> sizeof(p4dval_t)), \ >> sizeof(pgdval_t)) + 1) >> >> Would probably save stack space for 32bit arches? > > Do we really care about that? This is about a corner cases error reporting right > now. Fair enough, it just seemed like an obvious and simple (very minor) improvement. > >> >>> + >>> +static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size) >>> +{ >>> + if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) { >>> + snprintf(buf, buf_size, "overflow"); >>> + return; >>> + } >>> + >>> + switch (entry_size) { >>> + case sizeof(u32): >>> + snprintf(buf, buf_size, "%08x", *(const u32 *)entry); >>> + break; >>> + case sizeof(u64): >>> + snprintf(buf, buf_size, "%016llx", *(const u64 *)entry); >>> + break; >>> +#if defined(__SIZEOF_INT128__) >>> + case sizeof(u128): >>> + snprintf(buf, buf_size, "%016llx%016llx", >>> + (unsigned long long)(*(const u128 *)entry >> 64), >>> + (unsigned long long)*(const u128 *)entry); >>> + break; >>> +#endif >>> + default: >>> + snprintf(buf, buf_size, "unsupported"); >>> + break; >>> + } >>> +} >>> + >>> +#define ptval_to_str(buf, val) \ >>> + do { \ >>> + __auto_type __val = (val); \ >>> + \ >>> + ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val)); \ >>> + } while (0) >> >> I think arm64 code also does pte printing, which you also need to fix up for >> D128 support. Perhaps this could be moved to a header for reuse? > > We could do that as a second step, right? Sure, but why churn it twice? Anyway, no strong opinion, you're the boss :) > >> >> Also not sure if it's worth returning buf so that this pattern would be possible: >> >> char pmd_str[PTVAL_STR_MAX]; >> pr_err("pmd=%s\n, ptval_to_str(pmd_str, pmd_val(*pmdp)); >> >> Although perhaps that's a bit busy and should be discouraged... > > Yeah, let's not do that. >