From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935233AbcI3AXQ (ORCPT ); Thu, 29 Sep 2016 20:23:16 -0400 Received: from foss.arm.com ([217.140.101.70]:34610 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935085AbcI3AXK (ORCPT ); Thu, 29 Sep 2016 20:23:10 -0400 Date: Fri, 30 Sep 2016 01:13:40 +0100 From: Mark Rutland To: Laura Abbott Cc: AKASHI Takahiro , Ard Biesheuvel , David Brown , Will Deacon , Catalin Marinas , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Kees Cook , kernel-hardening@lists.openwall.com Subject: Re: [PATCH 1/3] arm64: dump: Make ptdump debugfs a separate option Message-ID: <20160930001339.GA4369@remoulade> References: <20160929213257.30505-1-labbott@redhat.com> <20160929213257.30505-2-labbott@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160929213257.30505-2-labbott@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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 > #include 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