From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1032798AbbKFICj (ORCPT ); Fri, 6 Nov 2015 03:02:39 -0500 Received: from mail-pa0-f49.google.com ([209.85.220.49]:34265 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031988AbbKFICh (ORCPT ); Fri, 6 Nov 2015 03:02:37 -0500 Date: Fri, 6 Nov 2015 16:02:33 +0800 From: Jiaxing Wang To: Steven Rostedt Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not compiled or initialized. Message-ID: <20151106080233.GA4174@cac> Mail-Followup-To: Steven Rostedt , linux-kernel@vger.kernel.org References: <20151102091637.0fcc05b2@gandalf.local.home> <1446599478-8723-1-git-send-email-hello.wjx@gmail.com> <20151104100339.583e1bc9@gandalf.local.home> <20151105052301.GA18152@cac> <20151105105432.3c0c9e3e@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151105105432.3c0c9e3e@gandalf.local.home> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 05, 2015 at 10:54:32AM -0500, Steven Rostedt wrote: > On Thu, 5 Nov 2015 13:23:01 +0800 > Jiaxing Wang wrote: > > > > > - /* > > > > - * As there may still be users that expect the tracing > > > > - * files to exist in debugfs/tracing, we must automount > > > > - * the tracefs file system there, so older tools still > > > > - * work with the newer kerenl. > > > > - */ > > > > - tr->dir = debugfs_create_automount("tracing", NULL, > > > > - trace_automount, NULL); > > > > - if (!tr->dir) { > > > > - pr_warn_once("Could not create debugfs directory 'tracing'\n"); > > > > - return ERR_PTR(-ENOMEM); > > > > + if (debugfs_initialized()) { > > > > + /* > > > > + * As there may still be users that expect the tracing > > > > + * files to exist in debugfs/tracing, we must automount > > > > + * the tracefs file system there, so older tools still > > > > + * work with the newer kerenl. > > > > + */ > > > > + traced = debugfs_create_automount("tracing", NULL, > > > > + trace_automount, NULL); > > > > + if (!traced) > > > > + pr_warn_once("Could not create debugfs directory 'tracing'\n"); > > > > > > This should return a warning, and please keep the tr->dir instead of > > > this new traced variable. > > Do you mean return ERR_PTR(-ENOMEM); when debugfs_create_automount() > > return NULL? > > Right. > > > As long as tracefs is initialized, we can make tracing_init_dentry() return > > NULL even if the debugfs automount point is not created(), and tracefs can > > still be populated. If tracing_init_dentry() returns error in this case, > > the caller of tracing_init_dentry() will not populate tracefs. > > But this is still a failure. tracing_init_dentry() now only mounts > tracefs on the debugfs/tracing directory. If it fails to do that when > debugfs is available, then it should fail, as it would break backward > compatibility with other tools. > > If debugfs is not configured in, then it should set tr->dir to > whatever (ENOMEM is fine), and return NULL. > > > > > > > > > } > > > > > > > > + tr->dir = TRACE_TOP_DIR_ENTRY; > > > > + > > > > > > Also, no need to add this, because if debugfs is not initialize, then > > > tr->dir would be ERR_PTR(-ENODEV), which still works as tr->dir is not > > > NULL. > > If we accept debugfs_create_automount() return NULL, tr->dir can still > > be NULL if we do tr->dir = debugfs_create_automount(). > > What's wrong with that? This function is only to automount debugfs now. > > Also, I think the first check should be: > > if (WARN_ON(!tracefs_initialized()) || > (IS_ENABLED(CONFIG_DEBUGFS) && > WARN_ON(!debugfs_initialized())) > return ERR_PTR(-ENODEV); > > Then we don't need the if (debugfs_initialized()) conditional. I will send you a new patch according to your suggestion, and if that is OK, I will send a seperate patch to Greg KH to add stub for debugfs_create_automount(). Thanks. > > -- Steve > > > > > > > > -- Steve > > > > > > > return NULL; > > > > } > > > > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/