thoughts on potential cleanup of semaphores?

["Robert P. J. Day" <rpjday@mindspring.com>]

  after submitting one patch related to semaphores and before i submit
any others, any thoughts whether any of the following clean-ups are
valid and/or worthwhile?  (some are admittedly simply aesthetic but
better aesthetics is never a bad thing.)


1) can all instances of sema_init() in the header files be simplified
based on the comment you can see in some of those header files?

========================
static inline void sema_init (struct semaphore *sem, int val)
{
/*
 *      *sem = (struct semaphore)__SEMAPHORE_INITIALIZER((*sem),val);
 *
 * i'd rather use the more flexible initialization above, but sadly
 * GCC 2.7.2.3 emits a bogus warning. EGCS doesnt. Oh well.
 */
        atomic_set(&sem->count, val);
        sem->sleepers = 0;
        init_waitqueue_head(&sem->wait);
}
========================

  one would think there's little value in retaining code that
accommodates something as old as GCC 2.7.2.3, but i'm not the expert
here.


2) [aesthetic] update all __SEMAPHORE_INITIALIZER initialization to
use C99-style structure initializers


3) i'm not a multi-arch wizard so is it true that all architectures
should have a struct semaphore with (approximately) the following
basic structure defined in their semaphore.h?

========================
struct semaphore {
        atomic_t count;
        int sleepers;
        wait_queue_head_t wait;
};
=======================

  you'd think so but there are some discrepancies.  in asm-frv, you
have

struct semaphore {
        unsigned                counter;
        spinlock_t              wait_lock;
        struct list_head        wait_list;

why "unsigned" and not "atomic_t"?  and why "counter" and not just
"count"?  (although, for the frv arch, that variable appears to be
unused except in a single place for debugging.)  and why a "struct
list_head" rather than what everyone else uses, a "wait_queue_head_t"?

i also note that some arches (m68knommu, m68k, cris) define a
"waking" variable instead of "sleepers".  is this supposed to
represent the same thing?  i haven't looked closely enough, sorry.


  and stepping back and looking at the bigger picture, it seems that
the semaphore.h files across all architectures are *almost* identical,
with a small number of differences, such as:

* some take a spinlock
* one (sparc) uses ATOMIC24_INIT rather than just ATOMIC_INIT

and there's probably a couple other things but, if these header files
are so similar, what about defining a single, generic semaphore.h
header file with a couple #ifdef's to handle the few possible
architecture-specific differences?  superficially, it doesn't look
like it would be that hard but that's the voice of youthful enthusiasm
speaking.

rday

p.s.  trying to condense all of the separate semaphore.h files into a
single, configurable one would also solve the problem of incorrect
documentation in some of them that is clearly the result of
cut-and-paste.  but i'm interested in what the experts have to say.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/