> You are not initializing right semaphores. Reading the code it seems that
> sema_init() has to be used only once the semaphore has just a sense.
> sema_init only change the level of the atomic counter but you could have
> wakers == ~0UL on the random stack ;). So it worked by luck all the time
> ;))
no. instead of working around a slightly broken kernel interface, i think
the right fix is to make sema_init() initialize _all_ semaphore fields.
This was a single field originally, but now they have grown and
sema_init() is used so rarely that it didnt show up immediately. It has to
do something like this:
#define sema_init(sem, val) \
(sem)->owner = (sem)->owner_depth = (sem)->waking = 0; \
(sem)->wait = NULL; \
(sem)->count = (val); \
wmb();
(wmb() is needed because after sema_init() every driver should be able to
assume that the semaphore is initialized in every CPU's view, and we also
have to be sure that GCC doesnt optimize writes across that point.)
runtime initialization of semaphores is perfectly OK, just setting the
counter field makes no sense at all.
[btw, if we are about to look at wmb() issues, i think atomic_set() doesnt
anymore need the 'fool GCC' stuff, we have a wmb() there which already
keeps GCC from moving writes across that point. The current code is sure
correct, just a bit redundant and slightly inefficient. (because it
completely fools GCC into not optimizing that piece of code, while we only
want to establish an optimization barrier)]
-- mingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/