lkml.org 
[lkml]   [2015]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH RESEND] tracing: Make tracing work when debugfs is not compiled or initialized.
    On Thu, Nov 05, 2015 at 10:54:32AM -0500, Steven Rostedt wrote:
    > On Thu, 5 Nov 2015 13:23:01 +0800
    > Jiaxing Wang <hello.wjx@gmail.com> 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/


    \
     
     \ /
      Last update: 2015-11-06 09:21    [W:2.302 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site