Re: Issues with "x86, um: switch to generic fork/vfork/clone" commit

From: Al Viro
Date: Mon Jan 21 2013 - 01:02:26 EST


On Sun, Jan 20, 2013 at 06:39:09PM -0800, Linus Torvalds wrote:

> And right now, that HAVE_SYSCALL_WRAPPERS does make it much harder to
> think about the header file changes.

Agreed.

> > FWIW, there's another bit of ugliness around that area - all these
> > #define __SC_BLAH3, etc., all of the same form. This stuff begs for
> > something like
> > #define __MAP1(m,t,a) m(t,a)
> > #define __MAP2(m,t,a,...) m(t,a) __MAP1(m,__VA_ARGS__)
> > #define __MAP3(m,t,a,...) m(t,a) __MAP2(m,__VA_ARGS__)
> > #define __MAP4(m,t,a,...) m(t,a) __MAP3(m,__VA_ARGS__)
> > #define __MAP5(m,t,a,...) m(t,a) __MAP4(m,__VA_ARGS__)
> > #define __MAP6(m,t,a,...) m(t,a) __MAP5(m,__VA_ARGS__)
> > #define __MAP(n,...) __MAP##n(__VA_ARGS__)
> > with __MAP(x,__SC_DECL,__VA_ARGS__) instead of __SC_DECL##x(__VA_ARGS__)
> > etc. in users...

... with missing commas added, of course.

> Well, I can see both sides. The above is the nice and dense
> declaration model with less duplication, but christ, it's hard for
> people to wrap their minds around unless they've seen it a million
> times. It really does take some getting used to, and the long-form can
> be easier to understand.

Umm... Even with
/*
* __MAP - apply a given macro to all syscall arguments.
* __MAP(n, m, t1, a1, ..., tn, an) will expand to
* m(t1,a1), m(t2,a2), ..., m(tn, an)
* Note that the first argument of __MAP must be equal to the number of
* type, name pairs in the list. The list itself (all arguments of __MAP
* starting with the 3rd one) is in the form we pass to SYSCALL_DEFINE<n>.
*/
slapped on top of it?

> That said, we have so many of those things now when it comes to the
> syscall stuff that the dense form seems to be called for just to be
> consistent.
>
> So go wild if you have the energy for it. I'm not going to pull that
> for 3.8, though.

No, that's obviously next cycle fodder, along with the sick tricks for
generating compat wrappers on s390 if Martin can live with those.

BTW, grep for asmlinkage; it's amazing how much cargo-culting is going
on with it ;-/ Some of the instances are syscalls yet to be converted
to SYSCALL_DEFINE<n>; even more of COMPAT_SYSCALL_DEFINE<n>-to-be. We
also have a bunch of declarations in syscalls.h and compat.h - those
are fine. _Some_ of the rest might be legitimate - ia64 and i386 have
non-trivial asmlinkage expansion and some (but not all) of arch/{x86,ia64}
instances do make sense. Not all of those - e.g. things like
FPU_divide_by_zero() have no business being regparm(0); they are only called
from C code and forcing their arguments on stack is a pure pessimization for
no reason whatsoever. Everything else in arch/* is magic green marker,
AFAICS...

There are some borderline cases - e.g. I'm not sure if having sys_recv
done *not* via SYSCALL_DEFINE() is deliberate; it might cut down on
some overhead (the sucker's calling sys_recvfrom(), which does normalizations,
which make normalizing in sys_recv() pointless). OTOH, sys_send *is*
done as SYSCALL_DEFINE, even though it ends up calling sys_sendto()...
--
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/