netdev - Re: [PATCH v3 6/7] venus: Make debug infrastructure more flexible

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:   Thu, 11 Jun 2020 15:33:45 -0700
From:   Joe Perches <joe@...ches.com>
To:     Jason Baron <jbaron@...mai.com>, jim.cromie@...il.com,
        Daniel Thompson <daniel.thompson@...aro.org>
Cc:     Stanimir Varbanov <stanimir.varbanov@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Documentation List <linux-doc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, linux-media@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-btrfs@...r.kernel.org,
        linux-acpi@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v3 6/7] venus: Make debug infrastructure more flexible

On Thu, 2020-06-11 at 17:59 -0400, Jason Baron wrote:
> 
> On 6/11/20 5:19 PM, jim.cromie@...il.com wrote:
> > trimmed..
> > 
> > > > > Currently I think there not enough "levels" to map something like
> > > > > drm.debug to the new dyn dbg feature. I don't think it is intrinsic
> > > > > but I couldn't find the bit of the code where the 5-bit level in struct
> > > > > _ddebug is converted from a mask to a bit number and vice-versa.
> > > > 
> > > > Here [1] is Joe's initial suggestion. But I decided that bitmask is a
> > > > good start for the discussion.
> > > > 
> > > > I guess we can add new member uint "level" in struct _ddebug so that we
> > > > can cover more "levels" (types, groups).
> > > 
> > > I don't think it is allocating only 5 bits that is the problem!

There were 6 unused bits in struct _ddebug;

The original idea was to avoid expanding the already somewhat
large struct _ddebug uses and the __verbose/__dyndbg section
that can have quite a lot of these structs.

I imagine adding another int or long wouldn't be too bad.

> > > The problem is that those 5 bits need not be encoded as a bitmask by
> > > dyndbg, that can simply be the category code for the message. They only
> > > need be converted into a mask when we compare them to the mask provided
> > > by the user.
> > > 

I also suggested adding a pointer to whatever is provided
by the developer so the address of something like
MODULE_PARM_DESC(variable, ...) can be also be used.

> > heres what I have in mind.  whats described here is working.
> > I'll send it out soon
> 
> Cool. thanks for working on this!

Truly, thank you both Jim and Stanimir.

Please remember that dynamic_debug is not required and
pr_debug should still work.

> >     API:
> > 
> >     - change pr_debug(...)  -->  pr_debug_typed(type_id=0, ...)
> >     - all existing uses have type_id=0
> >     - developer creates exclusive types of log messages with type_id>0
> >       1, 2, 3 are disjoint groups, for example: hi, mid, low

You could have a u8 for type if there are to be 3 classes

	bitmask
	level
	group by value

though I believe group by value might as well just be bitmask
and bool is_bitmask is enough (!is_bitmask would be level)

cheers, Joe