linux-kernel - Re: [PATCH v5 2/3] tracing: Add trace_total_entries() / trace_total_entries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190319105525.7686763b@gandalf.local.home>
Date:   Tue, 19 Mar 2019 10:55:25 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Daniel Thompson <daniel.thompson@...aro.org>,
        Ingo Molnar <mingo@...hat.com>,
        Jason Wessel <jason.wessel@...driver.com>,
        kgdb-bugreport@...ts.sourceforge.net,
        Brian Norris <briannorris@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 2/3] tracing: Add trace_total_entries() /
 trace_total_entries_cpu()

On Tue, 19 Mar 2019 07:45:38 -0700
Doug Anderson <dianders@...omium.org> wrote:

> > > +unsigned long trace_total_entries_cpu(struct trace_array *tr, int cpu)
> > > +{
> > > +     unsigned long total, entries;
> > > +
> > > +     if (!tr)
> > > +             tr = &global_trace;  
> >
> > This function is only ever called with tr set to NULL which means tr is
> > an argument looking for a user.
> >
> > I wouldn't mind except if this was following copying prior art to keep
> > the API feel the same but I can't find any other trace function where
> > the trace_array can be substituted for NULL. AFAICT all the existing
> > sites where global_trace is used will use it unconditionally.  

I'm in the process of making everything work better with tracing
instances. Which means, making all the APIs use the tr pointer (which is
the descriptor for the instance).

All new APIs added to the tracing infrastructure should reference a "tr"
pointer, and not use the global_trace directly. We can default it if tr
is NULL.

There are currently lots of cases in the code that do so, but I'm
trying to get rid of them.

> 
> Happy to change this if you guys want.  At the moment the
> trace_total_entries() comes straight from Steven's suggestion at:
> 
> https://lkml.kernel.org/r/20190315144130.1aa36931@gandalf.local.home
> 
> Ugh, but while looking at this it looks like I totally forgot one
> piece of feedback from Steven: to move the disabling of the tracing
> out so it's not re-enabled after counting but before dumping.  Not
> sure how I missed that.  :(
> 
> I'll wait to spin until I hear from Steven if he wants me to remove
> the "struct trace_array *tr" argument, then I'll post out a v6 with
> the fix I forgot...

Please keep the tr argument.

Thanks!

-- Steve