From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935365AbcI3BLy (ORCPT ); Thu, 29 Sep 2016 21:11:54 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:35266 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755519AbcI3BLr (ORCPT ); Thu, 29 Sep 2016 21:11:47 -0400 Subject: Re: [PATCH 1/3] arm64: dump: Make ptdump debugfs a separate option To: Mark Rutland References: <20160929213257.30505-1-labbott@redhat.com> <20160929213257.30505-2-labbott@redhat.com> <20160930001339.GA4369@remoulade> <20160930004852.GC4369@remoulade> 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 From: Laura Abbott Message-ID: Date: Thu, 29 Sep 2016 18:11:44 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20160930004852.GC4369@remoulade> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/29/2016 05:48 PM, Mark Rutland wrote: > On Thu, Sep 29, 2016 at 05:31:09PM -0700, Laura Abbott wrote: >> On 09/29/2016 05:13 PM, Mark Rutland wrote: >>> On Thu, Sep 29, 2016 at 02:32:55PM -0700, Laura Abbott wrote: >>>> +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? > > Sorry, I should have worded that better. > > I meant moving ptdump_register into ptdump_debugfs.c, perhaps renamed to make it > clear it's debugfs-specific. > > We could instead update existing users to call ptdump_debugfs_create() > directly, and have that call ptdump_initialize(), which could itself become a > staic inline in a header. Ah okay, I see what you are suggesting. ptdump_initialize should still happen regardless of debugfs status though so I guess ptdump_debugfs_create would just get turned into just ptdump_initialize which seems a little unclear. I'll come up with some other shed colors^W^Wfunction names. Thanks, Laura