Messages in this thread | | | From | Linus Torvalds <> | Date | Wed, 29 Mar 2023 09:50:39 -0700 | Subject | Re: [PATCH 4/7] sempahore: add a helper for a concurrency limiter |
| |
On Wed, Mar 29, 2023 at 2:19 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Arguably DEFINE_SEMAPHORE() should have the argument, as binary > semaphores are a special case, but then we gotta go and fix up all > users.
Using semaphores for just pure mutual exclusion used to be *the* most common use of it, which is why we didn't have an argument.
Then we got the mutexes, and now semaphores are almost entirely a legacy thing.
I think we should just make DEFINE_SEMAPHORE() take the number, and people who want a mutex should either put in the "1", or they should just use a mutex.
> /me git-greps a little.. Hmm, not too bad. > > How's this?
I'd actually prefer to not have that DEFINE_BINARY_SEMAPHORE() at all. It really shouldn't exist in this day and age.
It's not even less typing, ie
static DEFINE_SEMAPHORE(efivars_lock, 1);
is actually shorter than
static DEFINE_BINARY_SEMAPHORE(efivars_lock);
And what you actually *want* is
static DEFINE_MUTEX(efivars_lock);
and converting the up/down to mutex_unlock/mutex_lock.
So let's just make it clear that the only reason to use semaphores these days is for counting semaphores, and just make DEFINE_SEMAPHORE() take the number.
Strangely, sema_init() already does that, but I guess that's because some people really *do* use semaphores for concurrency control (ie I see things like
sema_init(&dc->in_flight, 64);
which is clearly using a semaphore in that very traditional way).
So ack on your patch, but don't bother with DEFINE_BINARY_SEMAPHORE().
Linus
| |