Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

[Rasmus Villemoes]

On Sat, Dec 12 2015, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:

> On Fri, Dec 11, 2015 at 11:16:24PM +0000, Al Viro wrote:
>
>> #define set_delayed_type(call, f, arg) \
>> 	sizeof(f(arg),0), \
>> 	__set_delayed_type(call, __closure_##f, (void *)arg)
>> 
>> That could be reused for timers with typechecking - we have a lot of timer
>> callbacks that start with casting the argument (unsigned long, not void *,
>> but that's not a big deal) to whatever it is that callback really wants,
>> with setup_timer() callers explicitly casting that whatever the callback
>> really wants to unsigned long.  Which, of course, defeats the typechecking
>> by both cc(1) and sparse(1)...
>> 
>> I still hope for better solution, though...  Comments?
>
> Hmm...
>
> #define set_delayed_type(call, f, arg)					\
>  	(sizeof(f(arg),0),						\
> 	__set_delayed_type(call, ({					\
> 		void _(void *__) {f((typeof(arg))(unsigned long)__);}	\
> 		_;}), (void *)arg))
>
> woult do nicely, but it's a gccism, and the one clang doesn't support
> ;-/

Careful. While this may appear to work, I'm pretty sure it would break
horribly when f is not the actual name of a function but a function
pointer. Not that it won't compile, but it would presumably make gcc
emit what they call a 'trampoline', a small piece of executable code
located on the stack, allowing the stub to access the containing
function's local variables (namely, f). I don't know if kernel stacks
are even executable, but even then it's a bad idea, once the stack frame
containing the trampoline is gone. All of this could easily happen down
the line when someone sees a bunch of similar code and decides to factor
that into a helper(), and then it works whenever gcc decides to inline
the helper to both call sites and otherwise, as the gcc docs put it,
"all hell breaks loose". IOWs, an accident waiting to happen.

One could probably ensure that the macro is always used with a
compile/link-time constant by putting in some trick like

static void *link_time_constant_please = f;

but even then I wouldn't be completely sure that gcc would always emit a
stub as simple as the one you showed.

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