linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* drivers/misc/habanalabs: atomic_t api usage inconsistencies
@ 2020-09-21 22:08 Shuah Khan
  2020-09-23  8:41 ` Oded Gabbay
  0 siblings, 1 reply; 3+ messages in thread
From: Shuah Khan @ 2020-09-21 22:08 UTC (permalink / raw)
  To: oded.gabbay, Arnd Bergmann, Greg Kroah-Hartman, obitton,
	lee.jones, oshpigelman
  Cc: Linux Kernel Mailing List, Shuah Khan

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: drivers/misc/habanalabs: atomic_t api usage inconsistencies
  2020-09-21 22:08 drivers/misc/habanalabs: atomic_t api usage inconsistencies Shuah Khan
@ 2020-09-23  8:41 ` Oded Gabbay
  2020-09-23 19:38   ` Shuah Khan
  0 siblings, 1 reply; 3+ messages in thread
From: Oded Gabbay @ 2020-09-23  8:41 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Ofir Bitton, Lee Jones,
	Omer Shpigelman, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: drivers/misc/habanalabs: atomic_t api usage inconsistencies
  2020-09-23  8:41 ` Oded Gabbay
@ 2020-09-23 19:38   ` Shuah Khan
  0 siblings, 0 replies; 3+ messages in thread
From: Shuah Khan @ 2020-09-23 19:38 UTC (permalink / raw)
  To: Oded Gabbay
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Ofir Bitton, Lee Jones,
	Omer Shpigelman, Linux Kernel Mailing List, Shuah Khan

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






^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-09-23 19:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21 22:08 drivers/misc/habanalabs: atomic_t api usage inconsistencies Shuah Khan
2020-09-23  8:41 ` Oded Gabbay
2020-09-23 19:38   ` Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).