[PATCH 1/3] arm64: dump: Make ptdump debugfs a separate option

Mark Rutland mark.rutland at arm.com
Thu Sep 29 17:13:40 PDT 2016


Hi,

On Thu, Sep 29, 2016 at 02:32:55PM -0700, Laura Abbott wrote:
> ptdump_register currently initializes a set of page table information and
> registers debugfs. There are uses for the ptdump option without wanting the
> debugfs options. Split this out to make it a separate option.
> 
> Signed-off-by: Laura Abbott <labbott at redhat.com>
> ---
>  arch/arm64/Kconfig.debug        |  6 +++++-
>  arch/arm64/include/asm/ptdump.h | 15 +++++++++++++--
>  arch/arm64/mm/Makefile          |  3 ++-
>  arch/arm64/mm/dump.c            | 30 +++++++++---------------------
>  arch/arm64/mm/ptdump_debugfs.c  | 33 +++++++++++++++++++++++++++++++++
>  5 files changed, 62 insertions(+), 25 deletions(-)
>  create mode 100644 arch/arm64/mm/ptdump_debugfs.c

As a heads-up, Ard has new ARM64_PTUMP user under drivers/firmware/efi queued
up in the EFI tree, which will also need fixing up. See commit d80448ac92b72051
("efi/arm64: Add debugfs node to dump UEFI runtime page tables") [1].

[...]

> +#include <linux/seq_file.h>
>  #include <linux/mm_types.h>

Nit: please keep headers in alphabetical order.

> -static void walk_pgd(struct pg_state *st, struct mm_struct *mm,
> +static void __walk_pgd(struct pg_state *st, struct mm_struct *mm,

Can we leave this name as-is? We didn't change walk_{pud,pmd,pte}, so this is
inconsistent, and we haven't reused the name.

[...]

> +int ptdump_register(struct ptdump_info *info, const char *name)
> +{
> +	ptdump_initialize(info);
> +	return ptdump_debugfs_create(info, name);
>  }

It feels like a layering violation to have the core ptdump code call the
debugfs ptdump code. Is there some reason this has to live here?

Other than the above points, this looks good to me.

Thanks,
Mark.

[1] https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=9d80448ac92b720512c415265597d349d8b5c3e8



More information about the linux-arm-kernel mailing list