> > have to be sure that GCC doesnt optimize writes across that point.)
>
> I think gcc can't reorder in a dangeros way. To run code that will start
> using the seamphore it has to return from the function, to call
> another function or to insert volatile inline asm.
no. the danger is not that GCC reorganizes the order of writes within the
initialization, but rather that GCC reorganizes flag modifications done
_after_ initializing the semaphore, which then arrive to another CPU not
in 'intended order'. eg:
semaphore_init(&mystruct.sem);
mystruct.ready = 1;
wmb();
if GCC optimizes '= 1' to happen before (or within) semaphore_init(), then
it might arrive to another CPU earlier than the code writer intended. Not
too likely to happen with 'classic' GCC, but i think it's already possible
with EGCS.
this is the very same issue you raised recently regarding sched.c.
> > runtime initialization of semaphores is perfectly OK, just setting the
> > counter field makes no sense at all.
>
> Infact. But my point is that the right way to runtime initialize a
> semaphore is just to use MUTEX_LOCKED and MUTEX. If you look at the code
> sema_init() was used _only_ as a performance optimization. You had many
> just zeroed (or just ready) structure and sema_init() was setting to 1 the
> only thing that had not to be zero. Probably is better to remove
> sema_init() to avoid people do mistakes? So it will be more clear that
> setting only .count is a performance hack?
>
> Here a patch that does that:
_no_. this is even more unsafe than your first patch because first, you
make the internal semaphore structure visible externally, second, you
cannot assume that other fields will be automatically zero. EGCS btw i
think already expands structure initializations for proper memset's and
thus merges away the 'superfluous' zeroing in sema_init(). third, even
when it's guaranteed to be zero, you again rely on internal details of our
semaphores. (who said all other fields will always be initialized to zero)
Fourth, being able to initialize dynamic variables is not 'just' a
performance optimization, it's a requirement.
> -static struct semaphore mfw_sema;
> +static struct semaphore mfw_sema = MUTEX;
> The last is a bugfix if the loader hasn't zeroed the .data segment before
> starting the code.
(this single change [it's not a bugfix assuming a correct sema_init()] is
a correct optimization because it's a constant variable so no need to
initialize runtime. But you might want to check with that particular
maintainer first wether he had other intentions.)
-- 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/