Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <3dfdda44-3564-1775-c844-f7a2680c9c58@linuxfoundation.org>
In-Reply-To: <3dfdda44-3564-1775-c844-f7a2680c9c58@linuxfoundation.org>
From:   Oded Gabbay <oded.gabbay@gmail.com>
Date:   Wed, 23 Sep 2020 11:41:50 +0300
Message-ID: <CAFCwf11Fg8gH6wMO5aPqwtNovgB153J2vNWdLF_zrua5b6Qetw@mail.gmail.com>
Subject: Re: drivers/misc/habanalabs: atomic_t api usage inconsistencies
To:     Shuah Khan <skhan@linuxfoundation.org>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Ofir Bitton <obitton@habana.ai>,
        Lee Jones <lee.jones@linaro.org>,
        Omer Shpigelman <oshpigelman@habana.ai>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Sep 22, 2020 at 1:08 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> All,
>
> While I was looking at the atomic_t api usages for an unrelated issue,
> I noticed free_slots_cnt in struct hl_cq incerment/decrement/reads are
> not consistent.
>
> atomic_inc() and atomic_set() are used, however instead of atomic_read()
> the value is referenced directly in
> drivers/misc/habanalabs/common/hw_queue.c
>
> hl_queue_add_ptr()
> atomic_t *free_slots = &hdev->completion_queue[q->cq_id].free_slots_cnt;
>
> hl_hw_queue_schedule_cs()
>
> atomic_t *free_slots = &hdev->completion_queue[i].free_slots_cnt;
>
> Any reason why this is necessary. I don't know that this is causing
> any problems, it is just odd that access is inconsistent.
>
> thanks,
> -- Shuah

Hi Shuah,
Thanks for taking notice of this issue :)
We will take a deeper look and fix the inconsistencies, although I
must say that we didn't notice any impact of this issue.
Thanks again.
Oded