lkml.org 
[lkml]   [2016]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/3] arm64: dump: Make ptdump debugfs a separate option
From
Date
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@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
>

\
 
 \ /
  Last update: 2016-09-30 02:31    [W:3.160 / U:1.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site