linux-kernel - Re: [PATCH v5 2/9] IB/core: Replace semaphore sm

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: <CA+55aFyd7d+C39ipC5XuZg1gJjJ+HFSgcPQU7Xb26Fsh6Z1_0A@mail.gmail.com>
Date:   Mon, 21 Nov 2016 09:57:51 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Binoy Jayan <binoy.jayan@...aro.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        Mustafa Ismail <mustafa.ismail@...el.com>,
        Lijun Ou <oulijun@...wei.com>,
        Nicholas Bellinger <nab@...ux-iscsi.org>,
        Leon Romanovsky <leonro@...lanox.com>,
        target-devel <target-devel@...r.kernel.org>,
        Tatyana E Nikolova <tatyana.e.nikolova@...el.com>,
        Doug Ledford <dledford@...hat.com>,
        Jenny Derzhavetz <jennyf@...lanox.com>,
        Sagi Grimberg <sagi@...mberg.me>,
        Sean Hefty <sean.hefty@...el.com>,
        Faisal Latif <faisal.latif@...el.com>,
        Hal Rosenstock <hal.rosenstock@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Wei Hu(Xavier)" <xavier.huwei@...wei.com>,
        Mark Brown <broonie@...nel.org>,
        Mark Bloch <markb@...lanox.com>,
        Steve Wise <swise@...ngridcomputing.com>,
        Ira Weiny <ira.weiny@...el.com>,
        Bart Van Assche <bart.vanassche@...disk.com>,
        Matan Barak <matanb@...lanox.com>,
        Sagi Grimberg <sagig@...lanox.com>
Subject: Re: [PATCH v5 2/9] IB/core: Replace semaphore sm_sem with an atomic wait

On Mon, Nov 21, 2016 at 8:52 AM, Arnd Bergmann <arnd@...db.de> wrote:
>
> Version of the series had replaced the semaphore with a completion
> here, which worked correctly, but one reviewer suggested using
> the wait_event() instead since it's confusing to have a completion
> starting out in 'completed' state.

Quite frankly, in that case just keep it as a semaphore.

So we have a few cases:

 - completions are *slow*.

   They are designed to be entirely synchronous, exactly so that you
can wait for a completion to finish, and then immediately free the
memory that the completion is in.

 - completions used fro mutual exclusion are *confusing*.

   Yes, they end up working as a counting semaphore, and yes, that's
by design, but at the same time, they are not _meant_ to be used as a
semaphore. The name matters. They are meant to be used the way they
are named: to be a "I'm done now" event. Don't use them for anything
else, because you *will* be confusing everybody else.

 - open-coding wait-queues are really fragile and are *not* meant for exclusion.

   They don't have lockdep checking, but more importantly, people
always invariably get the memory ordering wrong. And by "wrong" I mean
both "doesn't work" (because of insufficient memory ordering) and
"slow" (due to excessive memory ordering).

 - mutexes are good for exclusion.

   mutexes are both high-performance and non-confusing. Yes, they get
lockdep warnings, but that's usually a *good* thing. If you get
lockdep warnings from mutex use, you're almost certainly doing
something iffy.

   mutexes do have a subtle downside: exactly because they are fast,
they are not entirely synchronous. You can't use them for completion
style notifications if you are going to release the memory they are
allocated in.

   So mutexes need to be either statically allocated, or in
reference-counted allocations. That's so that the lifetime of the
memory is guaranteed to be cover all the users (including the wakers).

 - semaphores are "old-fashioned mutexes". A mutex is better than a
semaphore, but a semaphore is better than just about all the other
alternatives. There's nothing _wrong_ with using a semaphore per se.

In this case, either use a semaphore or a mutex. If you are doing
mutual exclusion, those are really the only two acceptable sleeping
models.

                Linus