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

Joe Perches
2020-06-11T22:34:01Z

On Thu, 2020-06-11 at 17:59 -0400, Jason Baron wrote:
>
> On 6/11/20 5:19 PM, jim.cromie@gmail.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