Re: [PATCH RFC 01/11] mm: introduce hw_pte_t for PTE table storage

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The header `<linux/pgtable_types.h>` incorrectly assumes `<asm/page.h>` universally defines `pte_t` and architecture opt-ins, leading to missing types on x86 and a structural inclusion hazard for opting in.
--

--- Patch [1]: [PATCH RFC 01/11] mm: introduce hw_pte_t for PTE table storage ---
commit de503ea03294c1c9976ce045b5e820dbc0f34eba
Author: Muhammad Usama Anjum <[email protected]>

mm: introduce hw_pte_t for PTE table storage

This commit introduces hw_pte_t as a generic name for a PTE table element.
It defines it as an alias of pte_t by default, allowing architectures to
retain their existing representation until they opt in to a distinct type.

> diff --git a/include/linux/pgtable_types.h b/include/linux/pgtable_types.h
> new file mode 100644
> index 0000000000000..ed5dba42a4f8c
> --- /dev/null
> +++ b/include/linux/pgtable_types.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_PGTABLE_TYPES_H
> +#define _LINUX_PGTABLE_TYPES_H
> +
> +#include <asm/page.h>
> +
> +#ifndef __HAVE_ARCH_HW_PTE_T
> +#define hw_pte_t pte_t
> +#endif

[Severity: Medium]
Is <asm/page.h> guaranteed to define pte_t on all architectures before it is
used here?

On x86, pte_t is defined in <asm/pgtable_types.h>, which is not included by
<asm/page.h>. This means pte_t might not be defined when the default hw_pte_t
alias is created.

Could this also create an inclusion hazard for architectures opting in?

If x86 were to opt into hw_pte_t by defining __HAVE_ARCH_HW_PTE_T in
<asm/pgtable_types.h>, any files including <linux/pgtable_types.h> first would
incorrectly alias hw_pte_t to pte_t. When <asm/pgtable_types.h> is
subsequently included, its explicit definition of hw_pte_t would macro-expand
to a conflicting redefinition of pte_t, breaking the build.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1