From: jim.cromie@gmail.com
To: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Stanimir Varbanov <stanimir.varbanov@linaro.org>,
Joe Perches <joe@perches.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linux Documentation List <linux-doc@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-btrfs@vger.kernel.org, linux-acpi@vger.kernel.org,
netdev@vger.kernel.org, Jason Baron <jbaron@akamai.com>
Subject: Re: [PATCH v3 6/7] venus: Make debug infrastructure more flexible
Date: Thu, 11 Jun 2020 15:19:21 -0600 [thread overview]
Message-ID: <CAJfuBxzE=A0vzsjNai_jU_16R_P0haYA-FHnjZcaHOR_3fy__A@mail.gmail.com> (raw)
In-Reply-To: <20200611121817.narzkqf5x7cvl6hp@holly.lan>
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!
>
> 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.
>
>
> Daniel.
heres what I have in mind. whats described here is working.
I'll send it out soon
commit 20298ec88cc2ed64269c8be7b287a24e60a5347e
Author: Jim Cromie <jim.cromie@gmail.com>
Date: Wed Jun 10 12:55:08 2020 -0600
dyndbg: WIP towards module->debugflags based callsite controls
There are *lots* of ad-hoc debug printing solutions in kernel,
this is a 1st attempt at providing a common mechanism for many of them.
Basically, there are 2 styles of debug printing:
- levels, with increasing verbosity, 1-10 forex
- bits/flags, independently controlling separate groups of dprints
This patch does bits/flags (with no distinction made yet between 2)
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
- !!type_id is just an additional callsite selection criterion
Qfoo() { echo module foo $* >/proc/dynamic_debug/control }
Qfoo +p # all groups, including default 0
Qfoo mflags 1 +p # only group 1
Qfoo mflags 12 +p # TBD[1]: groups 1 or 2
Qfoo mflags 0 +p # ignored atm TBD[2]
Qfoo mflags af +p # TBD[3]: groups a or f (10 or 15)
so patch does:
- add u32 debugflags to struct module. Each bit is a separate print-class.
- add unsigned int mflags into struct ddebug_query
mflags matched in ddebug_change
- add unsigned int type_id:5 to struct _ddebug
picks a single debugflag bit. No subclass or multitype nonsense.
nice and dense, packs with other members.
we will have a lot of struct _ddebugs.
- in ddebug_change()
filter on !! module->debugflags,
IFF query->module is given, and matches dt->mod_name
and query->mflags is given, and bitmatches module->debugflags
- in parse_query()
accept new query term: mflags $arg
populate query->mflags
arg-type needs some attention, but basic plumbing is there
WIP: not included:
- pr_debug_typed( bitpos=0, ....)
aka: pr_debug_class() or pr_debug_id()
the bitpos is 1<<shift, allowing a single type. no ISA relations.
this covers OP's high,mid,low case, many others
- no way to exersize new code in ddebug_change
need pr_debug_typed() to make a (not-null) typed callsite.
also no way to set module->debugflags
Im relying on:
cdf6d00696 dynamic_debug: don't duplicate modname in ddebug_add_module
which copies the ptr-val from module->name to dt->mod_name, which
allowed == tests instead of strcmp.
That equivalence and a (void*) cast in use of container_of() seem to
do the trick to get the module, then module->debugflags.
Notes:
1- A query ANDs all its query terms together, so Qfoo() above
requires both "module foo" AND all additional query terms given in $*
But since callsite type_id creates disjoint groups, "mflags 12" is
nonsense if it means groups 1 AND 2. Here, 1 OR 2 is meaningful, if
its not judged to be too confusing.
2- im not sure what this does atm, or should do
Qfoo mflags 0 +p # select only untyped ? or no flags check at all ?
3- since modflags has 32 bits, [1-9a-v] could select any single type_id
it is succinct, but arcane.
its a crude alphanumeric map for developers to make flags mnemonic
maybe query keyword should be mbits
next prev parent reply other threads:[~2020-06-11 21:19 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-09 10:45 [PATCH v3 0/7] Venus dynamic debug Stanimir Varbanov
2020-06-09 10:45 ` [PATCH v3 1/7] Documentation: dynamic-debug: Add description of level bitmask Stanimir Varbanov
2020-06-09 11:09 ` Matthew Wilcox
2020-06-09 11:16 ` Greg Kroah-Hartman
2020-06-09 16:58 ` Joe Perches
2020-06-09 17:42 ` Edward Cree
2020-06-09 17:56 ` Joe Perches
2020-06-09 18:08 ` Edward Cree
2020-06-10 6:31 ` Greg Kroah-Hartman
2020-06-10 6:35 ` Joe Perches
2020-06-10 7:09 ` Greg Kroah-Hartman
2020-06-10 7:24 ` Joe Perches
2020-06-10 10:29 ` Stanimir Varbanov
2020-06-10 12:26 ` Greg Kroah-Hartman
2020-06-09 10:45 ` [PATCH v3 2/7] dynamic_debug: Group debug messages by " Stanimir Varbanov
2020-06-09 12:27 ` Petr Mladek
2020-06-09 10:46 ` [PATCH v3 3/7] dev_printk: Add dev_dbg_level macro over dynamic one Stanimir Varbanov
2020-06-09 10:46 ` [PATCH v3 4/7] printk: Add pr_debug_level " Stanimir Varbanov
2020-06-09 11:12 ` Greg Kroah-Hartman
2020-06-09 10:46 ` [PATCH v3 5/7] venus: Add debugfs interface to set firmware log level Stanimir Varbanov
2020-06-09 11:12 ` Greg Kroah-Hartman
2020-06-11 11:51 ` Stanimir Varbanov
2020-06-09 10:46 ` [PATCH v3 6/7] venus: Make debug infrastructure more flexible Stanimir Varbanov
2020-06-09 11:14 ` Greg Kroah-Hartman
2020-06-10 13:29 ` Stanimir Varbanov
2020-06-10 13:37 ` Greg Kroah-Hartman
2020-06-10 19:49 ` Joe Perches
2020-06-10 20:23 ` Joe Perches
2020-06-11 6:26 ` Greg Kroah-Hartman
2020-06-11 6:42 ` Joe Perches
2020-06-11 10:52 ` Daniel Thompson
2020-06-11 11:31 ` Stanimir Varbanov
2020-06-11 12:18 ` Daniel Thompson
2020-06-11 21:19 ` jim.cromie [this message]
2020-06-11 21:59 ` Jason Baron
2020-06-11 22:33 ` Joe Perches
2020-06-12 0:08 ` jim.cromie
2020-06-10 18:32 ` WIP generic module->debug_flags and dynamic_debug jim.cromie
2020-06-10 20:24 ` Joe Perches
2020-06-11 11:26 ` Rasmus Villemoes
2020-06-11 14:09 ` jim.cromie
2020-06-09 10:46 ` [PATCH v3 7/7] venus: Add a debugfs file for SSR trigger Stanimir Varbanov
2020-06-09 11:13 ` [PATCH v3 0/7] Venus dynamic debug Matthew Wilcox
2020-06-09 16:03 ` Randy Dunlap
2020-06-09 16:49 ` Joe Perches
2020-06-09 21:21 ` jim.cromie
2020-06-09 22:23 ` Joe Perches
2020-06-10 1:58 ` Joe Perches
2020-06-10 3:10 ` jim.cromie
2020-06-09 16:40 ` Joe Perches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJfuBxzE=A0vzsjNai_jU_16R_P0haYA-FHnjZcaHOR_3fy__A@mail.gmail.com' \
--to=jim.cromie@gmail.com \
--cc=daniel.thompson@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=jbaron@akamai.com \
--cc=joe@perches.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=stanimir.varbanov@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.