linux-kernel - Re: [PATCH 4/7] sempahore: add a helper for a concurrency limiter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whF6Ta_KcJP2eC78+Mstv+vAku8ATRMbv98sf9VhdvySQ@mail.gmail.com>
Date:   Wed, 29 Mar 2023 09:50:39 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Luis Chamberlain <mcgrof@...nel.org>, david@...hat.com,
        patches@...ts.linux.dev, linux-modules@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org, pmladek@...e.com,
        petr.pavlu@...e.com, prarit@...hat.com, gregkh@...uxfoundation.org,
        rafael@...nel.org, christophe.leroy@...roup.eu, tglx@...utronix.de,
        song@...nel.org, rppt@...nel.org, willy@...radead.org,
        vbabka@...e.cz, mhocko@...e.com, dave.hansen@...ux.intel.com
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@...radead.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