Re: [PATCH v2 5/5] mm: ptep_deref() conversion

[SeongJae Park <sj@kernel.org>]

On Thu, 18 May 2023 12:07:27 +0100 Ryan Roberts <ryan.roberts@arm.com> wrote:

> Convert all instances of direct pte_t* dereferencing to instead use
> ptep_deref() helper. By default, the helper does a direct dereference as
> before, but it can (and will) be overridden by the architecture to fully
> encapsulate the contents of the pte. Arch code is deliberately not
> converted, as the arch code knows best.
> 
> Conversion was done using Coccinelle:
> 
> ----
> 
> // $ make coccicheck \
> //          COCCI=ptepderef.cocci \
> //          SPFLAGS="--include-headers" \
> //          MODE=patch
> 
> virtual patch
> 
> @ depends on patch @
> pte_t *v;
> @@
> 
> - *v
> + ptep_deref(v)
> 
> ----
> 
> Then reviewed and hand-edited to avoid multiple unnecessary calls to
> ptep_deref(), instead opting to store the result of a single in a
> variable, where it is correct to do so. This will benefit arch-overrides
> that may be more complex than a simple (optimizable) pointer
> dereference.
> 
> Included is a fix for an issue in an earlier version of this patch that
> was pointed out by kernel test robot. The issue arose because config
> MMU=n elides definition of the ptep helper functions, including
> ptep_deref(). HUGETLB_PAGE=n configs still define a simple
> huge_ptep_clear_flush() for linking purposes, which dereferences the
> ptep. So when both configs are disabled, this caused a build error
> because ptep_deref() is not defined. Fix by continuing to do a direct
> dereference when MMU=n. This is safe because for this config the arch
> code cannot be trying to virtualize the ptes because none of the ptep
> helpers are defined.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/oe-kbuild-all/202305120142.yXsNEo6H-lkp@intel.com/
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
[...]
>  mm/damon/ops-common.c                         |   2 +-
>  mm/damon/paddr.c                              |   2 +-
>  mm/damon/vaddr.c                              |  10 +-

For above mm/damon/ part,

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ