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

[Laura Abbott]

On 09/29/2016 05:13 PM, Mark Rutland wrote:
> 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].
>
> [...]
>

I'll take a look at that, thanks for the pointer!

>> +#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.
>

Yes, I think this is a relic of earlier refactoring attempts.

> [...]
>
>> +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?
>

Which 'this' are you referring to here? Are you suggesting moving
the ptdump_register elsewhere or moving the debugfs create elsewhere?

> 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
>