linux-kernel - Re: [PATCH v1] dynamic_debug: add support for logs destination

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]

Date:   Wed, 4 Oct 2023 12:55:01 +0200
From:   Łukasz Bartosik <lb@...ihalf.com>
To:     jim.cromie@...il.com
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Jason Baron <jbaron@...mai.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        Guenter Roeck <groeck@...gle.com>,
        Yaniv Tzoreff <yanivt@...gle.com>,
        Benson Leung <bleung@...gle.com>, linux-kernel@...r.kernel.org,
        upstream@...ihalf.com,
        Vincent Whitchurch <vincent.whitchurch@...s.com>
Subject: Re: [PATCH v1] dynamic_debug: add support for logs destination

wt., 3 paź 2023 o 22:54 <jim.cromie@...il.com> napisał(a):
>
> On Tue, Oct 3, 2023 at 1:57 PM Steven Rostedt <rostedt@...dmis.org> wrote:
> >
> > On Mon, 2 Oct 2023 14:49:20 -0600
> > jim.cromie@...il.com wrote:
> >
> > > hi Lukasz,
> > >
> > > sorry my kernel-time has been in my own trees.
> > >
> > > What I dont understand is why +T is insufficient.
> > >

We would like to be able to separate debug logs from different
subsystem (e.g. thunderbolt and usbcore).
With +T it is not possible because all debug logs will land in the same bucket.

> > > IIUC, tracefs is intended for production use.
> > > thats why each event can be enabled / disabled
> > > - to select and minimize whats traced, and not impact the system
> > >
> > > and +T  can forward all pr_debugs to trace,
> > > (by 1-few trace events defined similarly to others)
> > > or very few, giving yet another selection mechanism
> > > to choose or eliminate specific pr-debugs and reduce traffic to
> > > interesting stuff.
> > >
> > > Once your debug is in the trace-buf,
> > > shouldnt user-space be deciding what to do with it ?
> > > a smart daemon could leverage tracefs to good effect.
> > >

Yes, a daemon could separate the debug logs but IMHO it is much
easier to separate logs by sending them directly from a given subsystem
to a separate trace instance. My proposal allows to configure different
trace instance as destination for each callsite.

> > > IMO the main value of +T is that it allows feeding existing pr_debugs
> > > into the place where other trace-data is already integrated and managed.
> > >
> > > At this point, I dont see any extra destination handling as prudent.
> > >
> >
> >
> > I'm fine with either approach. I kind of like the creation of the instance,
> > as that allows the user to keep this debug separate from other tracing
> > going on. We are starting to have multiple applications using the tracing
> > buffer (although most are using instances, which is why I'm trying to make
> > them lighter weight with the eventfs code).
> >
> > -- Steve
> >

Steve, thanks for commenting from the trace perspective.

>
>
> Ok Im starting to grasp that multiple instances are good
> (and wondering how I didnt notice)
>
> What doesnt thrill me is the new _ddebug field, it enlarges the footprint.
>

Yes it increases _ddebug structure by a pointer size.

> can you make it go away ?

I implemented my proposal with flexibility in mind so that if someone
would like to add
another destination in the future it should be easy to do. I
understand that adding a pointer
to the _ddebug structure increases footprint size that's why I also
added CONFIG_DYNAMIC_DEBUG_DST
kernel configuration option in order to enable/disable this functionality.

> I have some thoughts ..

Please share your thoughts. I'm sure we can come to an agreement how
to incorporate both +T and my proposal.

Thanks,
Lukasz