Re: [patch 1/9] Conditional Calls - Architecture Independent Code

Mathieu Desnoyers
2007-06-05T18:40:28Z

* Andi Kleen (andi@firstfloor.org) wrote:
> Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> writes:
>
> > +struct __cond_call_struct {
>
> Calling structs *_struct is severly deprecated and will cause some people
> to make fun of your code.
>

ok

>
> > + const char *name;
> > + void *enable;
> > + int flags;
> > +} __attribute__((packed));
>
> The packed doesn't seem to be needed. There will be padding at
> the end anyways because the next one needs to be aligned.
>
ok

> > +
> > +
> > +/* Cond call flags : selects the mechanism used to enable the conditional calls
> > + * and prescribe what can be executed within their function. This is primarily
> > + * used at reentrancy-unfriendly sites. */
> > +#define CF_OPTIMIZED (1 << 0) /* Use optimized cond_call */
> > +#define CF_LOCKDEP (1 << 1) /* Can call lockdep */
> > +#define CF_PRINTK (1 << 2) /* Probe can call vprintk */
> > +#define CF_STATIC_ENABLE (1 << 3) /* Enable cond_call statically */
>
> Why is that all needed? Condcall shouldn't really need to know anything
> about all this. They're just a fancy conditional anyways -- and you don't
> tell if() that it may need to printk.
>
> Please consider eliminating.
>

I will remove the STATIC_ENABLE and the PRINTK, but I will leave the
CF_LOCKDEP and CF_OPTIMIZED there: they are required to let the generic
version be selected in contexts where a breakpoint cannot be used on
x86 (especially when placing a cond_call within lockdep.c code or any
code that could not afford to fall into a breakpoint handler).

>
>
> > +#define _CF_NR 4
>
>
> > +
> > +#ifdef CONFIG_COND_CALL
> > +
> > +/* Generic cond_call flavor always available.
> > + * Note : the empty asm volatile with read constraint is used here instead of a
> > + * "used" attribute to fix a gcc 4.1.x bug. */
>
> What gcc 4.1 bug?
>

Please see

http://www.ecos.sourceware.org/ml/systemtap/2006-q4/msg00146.html

for Jeremy Fitzhardinge's comment on the issue. I will add some comments
in the code.

Mathieu