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

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

* 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

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68