Messages in this thread | | | Subject | Re: [PATCH 1/3] arm64: dump: Make ptdump debugfs a separate option | From | Laura Abbott <> | Date | Thu, 29 Sep 2016 17:31:09 -0700 |
| |
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 >
| |