Re: [PATCH 1/19] MUTEX: Introduce simple mutex implementation

David Howells
2005-12-13T13:32:59Z

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > (5) Redirects the following to apply to the new mutexes rather than the
> > traditional semaphores:
> >
> > down()
> ...
>
> And you've audited every occurence ?

Outside of the arch directories, yes; but I don't know that I've made the
correct decision in 100% of the cases.

I've changed some of the uses into completions, and found about a dozen or so
uses of counting semaphores; but the vast majority of occurrences seem to be
wanting mutex behaviour.

> It seems to me it would be far far saner to define something like
>
> sleep_lock(&foo)
> sleep_unlock(&foo)
> sleep_trylock(&foo)

Which would be a _lot_ more work. It would involve about ten times as many
changes, I think, and thus be more prone to errors.

> Its then obvious what it does, you don't randomly break other drivers you've
> not reviewed and the interface is intuitive rather than obfuscated.

I've attempted to review everything in 2.6.15-rc5 outside of most of the archs.
I can't easily modify any driver not contained in that tarball, but at least
the compiler will barf and force a review.

> It won't take long for people to then change the name of the performance
> critical cases and the others will catch up in time.

It took about ten hours to go through the declarations of struct semaphore and
review them; I hate to think how long it'd take to go through all the ups and
downs too.

> It also saves breaking every piece of out of tree kernel code for now
> good reason.

But my patch means the changes required are in the most cases minimal: just
changing struct semaphore to struct mutex is sufficient for the vast majority
of cases.

Your way requires a lot more work, both in the tree and out of it.

David