linux-kernel - Re: [PATCH v4 10/10] IB/mlx5: Simplify completion into a wait

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: <CAHv-k_97o9eYODRgkWa-NodXPWf+hJ5M9+hHrkm=rn9p6cY9Jw@mail.gmail.com>
Date:   Thu, 3 Nov 2016 11:33:04 +0530
From:   Binoy Jayan <binoy.jayan@...aro.org>
To:     Sagi Grimberg <sagi@...mberg.me>, Leon Romanovsky <leon@...nel.org>
Cc:     Doug Ledford <dledford@...hat.com>,
        Sean Hefty <sean.hefty@...el.com>,
        Hal Rosenstock <hal.rosenstock@...il.com>,
        Arnd Bergmann <arnd@...db.de>, linux-rdma@...r.kernel.org,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 10/10] IB/mlx5: Simplify completion into a wait_event

Hi,

On 31 October 2016 at 02:47, Sagi Grimberg <sagi@...mberg.me> wrote:

> How is this simpler?

It is simpler in the sense that it is a light weight primitive and that only
one thread waits on the event here. In our case since 'umr_context.done'
is an "on stack" variable, and has only one thread waiting on that event,
no race conditions occur. So, we do not need completions here which
are usually used to provide a race-free but easy-to-use solution involving
multiple threads waiting on an event.

>>  enum ib_wc_status {
>> +       IB_WC_STATUS_NONE = -1,
>>         IB_WC_SUCCESS,
>>         IB_WC_LOC_LEN_ERR,
>>         IB_WC_LOC_QP_OP_ERR,
>>
>
> Huh? Where did this bogus status came from? IMHO, this is polluting
> the verbs interface for no good reason at all, sorry.

context->status is initialized to -1 in the following code, so I just thought of
replacing it with a name.

static inline void mlx5_ib_init_umr_context(struct mlx5_ib_umr_context *context)
 {
        context->cqe.done = mlx5_ib_umr_done;
-       context->status = -1;
-       init_completion(&context->done);
+       context->status = IB_WC_STATUS_NONE;
+       init_waitqueue_head(&context->wq);
 }

Thanks,
Binoy