lkml.org 
[lkml]   [2015]   [Nov]   [5]   [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, 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.

-- Steve


> >
> > -- Steve
> >
> > > return NULL;
> > > }
> > >
> >



\
 
 \ /
  Last update: 2015-11-05 17:01    [W:0.119 / U:2.572 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site