Re: [PATCH v8 06/15] mm: add hw_pte_val for HW PTE storage

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mm: hw_pte_val expands to rvalue breaking lvalue API contract
--

commit 5410dae6e74b38457df98b3df5924d0050eb76a9
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

mm: add hw_pte_val for HW PTE storage

This commit adds a hw_pte_val() macro to expose an lvalue for hardware PTE
storage. It accesses the wrapper's __pte member when hw_pte_t is distinct,
and uses pte_val() directly otherwise.

> diff --git a/include/linux/pgtable_types.h b/include/linux/pgtable_types.h
> index d6c5a7548550b..ee4eace5c3e1c 100644
> --- a/include/linux/pgtable_types.h
> +++ b/include/linux/pgtable_types.h
> @@ -9,9 +9,13 @@
>  #ifdef CONFIG_ARCH_HAS_HW_PTE_T
>  typedef struct __hw_pte_t { pte_t __pte; } hw_pte_t;
>  #define __pte_from_hw(pte)	((pte).__pte)
> +
> +#define hw_pte_val(x)  pte_val((x).__pte)

[Severity: Low]
The commit message states that hw_pte_val() is meant to expose an lvalue.
However, on architectures where pte_val() is implemented as a static inline
function rather than a macro (such as x86), doesn't pte_val() evaluate to
an rvalue instead? 

Will taking the address of hw_pte_val() in generic code yield an "lvalue
required" compiler error on those architectures?

>  #else
>  #define hw_pte_t pte_t
>  #define __pte_from_hw(pte)	(pte)
> +
> +#define hw_pte_val(x)  pte_val(x)

[Severity: Low]
Similarly, does this also evaluate to an rvalue on architectures where
pte_val() is a static inline function, breaking the expected lvalue API
contract?

>  #endif
>  
>  #endif /* !__ASSEMBLY__ */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791365932.git.agordeev@linux.ibm.com?part=6