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;
Subject: Re: drivers/misc/habanalabs: atomic_t api usage inconsistencies
To:     Oded Gabbay <oded.gabbay@gmail.com>
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>,
        Shuah Khan <skhan@linuxfoundation.org>
References: <3dfdda44-3564-1775-c844-f7a2680c9c58@linuxfoundation.org>
 <CAFCwf11Fg8gH6wMO5aPqwtNovgB153J2vNWdLF_zrua5b6Qetw@mail.gmail.com>
From:   Shuah Khan <skhan@linuxfoundation.org>
Message-ID: <07dd76cf-adb4-f7c4-2f33-85f08de95404@linuxfoundation.org>
Date:   Wed, 23 Sep 2020 13:38:27 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <CAFCwf11Fg8gH6wMO5aPqwtNovgB153J2vNWdLF_zrua5b6Qetw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/23/20 2:41 AM, Oded Gabbay wrote:
> 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.

If you haven't noticed any problems, it could be that this variable
doesn't need to atomic or you haven't run into it yet?

thanks,
-- Shuah