[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160930001339.GA4369@remoulade>
Date: Fri, 30 Sep 2016 01:13:40 +0100
From: Mark Rutland <mark.rutland@....com>
To: Laura Abbott <labbott@...hat.com>
Cc: AKASHI Takahiro <takahiro.akashi@...aro.org>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
David Brown <david.brown@...aro.org>,
Will Deacon <will.deacon@....com>,
Catalin Marinas <catalin.marinas@....com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Kees Cook <keescook@...omium.org>,
kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH 1/3] arm64: dump: Make ptdump debugfs a separate option
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@...hat.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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
