* Alex Shi <alex.shi@xxxxxxxxx> wrote: > On Wed, 2012-03-07 at 14:39 +0100, Ingo Molnar wrote: > > * Alex Shi <alex.shi@xxxxxxxxx> wrote: > > > > > > I think the check should be (__alignof__(lock) < > > > > __alignof__(rwlock_t)), otherwise it will still pass when > > > > you have structure with attribute((packed,aligned(2))) > > > > > > reasonable! > > > > > > >> 1, it is alignof bug for default gcc on my fc15 and Ubuntu 11.10 etc? > > > >> > > > >> struct sub { > > > >> int raw_lock; > > > >> char a; > > > >> }; > > > >> struct foo { > > > >> struct sub z; > > > >> int slk; > > > >> char y; > > > >> }__attribute__((packed)); > > > >> > > > >> struct foo f1; > > > >> > > > >> __alignof__(f1.z.raw_lock) is 4, but its address actually can align on > > > >> one byte. > > > > > > > > That looks like correct behavior, because the alignment of > > > > raw_lock inside of struct sub is still 4. But it does mean > > > > that there can be cases where the compile-time check is not > > > > sufficient, so we might want the run-time check as well, at > > > > least under some config option. > > > > > > what's your opinion of this, Ingo? > > > > Dunno. How many real bugs have you found via this patch? > > None. Guess stupid code was shot in lkml reviewing. But if the > patch in, it is helpful to block stupid code in developing. The question is, if in the last 10 years not a single such case made it through to today's 15 million lines of kernel code, why should we add the check now? If it was a simple build time check then maybe, but judging by the discussion it does not seem so simple, does it? Thanks, Ingo