Messages in this thread | | | Date | Mon, 25 Jan 1999 16:14:24 +0100 (CET) | From | MOLNAR Ingo <> | Subject | Re: down_interruptible and timers |
| |
On Mon, 25 Jan 1999, Andrea Arcangeli wrote:
> 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/
| |