linux-kernel - Re: [PATCH 1/4] dynamic_debug: consolidate repetitive struct

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: <20110912150011.GD2555@redhat.com>
Date:	Mon, 12 Sep 2011 11:00:14 -0400
From:	Jason Baron <jbaron@...hat.com>
To:	Jim Cromie <jim.cromie@...il.com>
Cc:	Joe Perches <joe@...ches.com>,
	Andrew Morton <akpm@...ux-foundation.org>, gregkh@...e.de,
	Bart Van Assche <bvanassche@....org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] dynamic_debug: consolidate repetitive struct _ddebug
 descriptor definitions

On Fri, Sep 09, 2011 at 01:23:43PM -0600, Jim Cromie wrote:
> On Fri, Sep 9, 2011 at 4:31 AM, Bart Van Assche <bvanassche@....org> wrote:
> > On Fri, Sep 9, 2011 at 6:02 AM, Joe Perches <joe@...ches.com> wrote:
> >> On Thu, 2011-09-08 at 20:42 -0700, Andrew Morton wrote:
> >> > On Thu, 08 Sep 2011 19:13:16 -0700 Joe Perches <joe@...ches.com> wrote:
> >> > > On Thu, 2011-09-08 at 16:52 -0700, Andrew Morton wrote:
> >> > > > On Tue, 30 Aug 2011 14:28:41 -0400
> >> > > > Jason Baron <jbaron@...hat.com> wrote:
> >> > > > > Replace the repetitive struct _ddebug descriptor definitions with
> >> > > > > a new DECLARE_DYNAMIC_DEBUG_META_DATA(name, fmt) macro.
> >> > > > > +#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt)              \
> >> > > > > +       static struct _ddebug __used __aligned(8)               \
> >> > > > > +       __attribute__((section("__verbose"))) name = {          \
> >> > > > > +               .modname = KBUILD_MODNAME,                      \
> >> > > > > +               .function = __func__,                           \
> >> > > > > +               .filename = __FILE__,                           \
> >> > > > > +               .format = (fmt),                                \
> >> > > > > +               .lineno = __LINE__,                             \
> >> > > > > +               .flags =  _DPRINTK_FLAGS_DEFAULT,               \
> >> > > > > +               .enabled = false,                               \
> >> > > > > +       }
> >> > > > <anal>That macro implements a definition, not a declaration</anal>
> >> > > Andrew, that's not quite true
> >> > It's precisely true.
> >> Not according to the c99 standard section 6.7
> >
> > Have you read that paragraph ? This is what I found in paragraph 6.7,
> > which confirms Andrews interpretation:
> >
> > <quote>
> > A declaration speciﬁes the interpretation and attributes of a set of
> > identiﬁers. A deﬁnition of an identiﬁer is a declaration for that
> > identiﬁer that:
> > - for an object, causes storage to be reserved for that object;
> > - for a function, includes the function body;
> > - for an enumeration constant or typedef name, is the (only)
> > declaration of the identiﬁer.
> > </quote>
> >
> > Bart.
> >
> 
> 
> I hesitate to churn this more (I have patchset to go on top of all this) but
> 
> Id like to see an INIT_DYNAMIC_DEBUG_METADATA,
> along with ability to expose the descriptor.
> 
> This would support pr_dbg_cont(), by letting it see/reuse
> the same descriptor that controls the pr_debug that started
> the multi-call message.  While this defeats the "atomicity"
> of buffering the entire message before calling printk,
> it does so only for the actual uses of KERN_CONT.
> 
> It also allows for "lite" usage of dynamic-debug,
> including 1..few descriptor per file or module to control all debug printing.
> As outlined, this "lite" usage is determined by the coder,
> it would be cool if it were more configurable than that,
> but I dont see how that would work atm.
> 
> 

We can expose the descriptor, but I that can wait for a folow-up
patchset, especially, since we don't have any consumers at the moment.

> Now that the worms have escaped the can, one other thought:
> 	unsigned int lineno:24;
> allows for insanely large files.  The largest in the tree is 29k,
> 16 bits would cover all files likely to be accepted in the future.
> Even allowing for never-to-be-submitted machine-generated code,
> Id think 18 bits would suffice.  ~250k lines should be enough ;-)
> 
> Given that my patchset adds flags-filtering
> ( echo mt+p > $CONTROL )
> The availability of user-flags, which do nothing but mark callsites,
> has some value - user can mark arbitrary sets of callsites,
> then enable/disable them as a group or subgroup:
> 
> echo function foo +x > $CONTROL
> ...
> echo x+p > $CONTROL
> echo y-p > $CONTROL
> echo z+p > $CONTROL
> echo yz-p > $CONTROL
> 
> Since since it works with module/file/function filtering, 3-4 user
> flags should be plenty.

makes sense...this can also be done is follow-on patchset...

thanks,

-Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/