public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* regarding use of various cmpxchg* API
@ 2014-06-05 22:44 Pranith Kumar
  2014-06-06  7:29 ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Pranith Kumar @ 2014-06-05 22:44 UTC (permalink / raw)
  To: peterz, mingo; +Cc: linux-kernel

Hi Peter, Ingo,

I see that in the drivers there are the following types of cmpxchg API:

__cmpxchg64()
atomic_cmpxchg()
atomic64_cmpxchg()
cmpxchg()
atomic_long_cmpxchg()
cmpxchg_local()
local_cmpxchg()(in kernel/events/ring_buffer.c)
this_cpu_cmpxchg()

Since cmpxchg() internally handles the width, do you think it makes sense to replace the above uses of cmpxchg with the document API (cmpxchg, atomic_cmpxchg, cmpxchg_local)?

I am willing to do this and wanted to know if it something you think is worth pursuing.

Regards,
Pranith

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

* Re: regarding use of various cmpxchg* API
  2014-06-05 22:44 regarding use of various cmpxchg* API Pranith Kumar
@ 2014-06-06  7:29 ` Peter Zijlstra
  2014-06-06 14:17   ` Pranith Kumar
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2014-06-06  7:29 UTC (permalink / raw)
  To: Pranith Kumar; +Cc: mingo, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On Thu, Jun 05, 2014 at 06:44:37PM -0400, Pranith Kumar wrote:
> Hi Peter, Ingo,
> 
> I see that in the drivers there are the following types of cmpxchg API:
> 
> __cmpxchg64()

That shouldn't really be used, and its only used in intel-iommu.c.

> atomic_cmpxchg()
> atomic64_cmpxchg()
> atomic_long_cmpxchg()

They're part of the atomic*_t family and should be used on their
respective types, no point in changing that.

> local_cmpxchg()(in kernel/events/ring_buffer.c)

This is part of the local_t API, also no real point to change that.

> this_cpu_cmpxchg()

This is part of the this_cpu*() family, and there's no way you can
generate the same code using the below to.

> cmpxchg()
> cmpxchg_local()

Which leaves these, which are used in all other cases.

> Since cmpxchg() internally handles the width, do you think it makes
> sense to replace the above uses of cmpxchg with the document API
> (cmpxchg, atomic_cmpxchg, cmpxchg_local)?
> 
> I am willing to do this and wanted to know if it something you think
> is worth pursuing.

Don't think that's useful. If you really want to go do something, try
the annotation I suggested to get the parisc/sparc32 things correct
again. Add the __atomic sparse address space and the store()/load()
accessors.

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: regarding use of various cmpxchg* API
  2014-06-06  7:29 ` Peter Zijlstra
@ 2014-06-06 14:17   ` Pranith Kumar
  2014-06-06 14:30     ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Pranith Kumar @ 2014-06-06 14:17 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: mingo, linux-kernel

On 06/06/2014 03:29 AM, Peter Zijlstra wrote:

> 
> Don't think that's useful. If you really want to go do something, try
> the annotation I suggested to get the parisc/sparc32 things correct
> again. Add the __atomic sparse address space and the store()/load()
> accessors.
> 

OK. I am not sure what you are referring to here, just started looking at the kernel again. Any pointers to this accessors thing?

--
Pranith

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

* Re: regarding use of various cmpxchg* API
  2014-06-06 14:17   ` Pranith Kumar
@ 2014-06-06 14:30     ` Peter Zijlstra
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2014-06-06 14:30 UTC (permalink / raw)
  To: Pranith Kumar; +Cc: mingo, linux-kernel

On Fri, Jun 06, 2014 at 10:17:20AM -0400, Pranith Kumar wrote:
> On 06/06/2014 03:29 AM, Peter Zijlstra wrote:
> 
> > 
> > Don't think that's useful. If you really want to go do something, try
> > the annotation I suggested to get the parisc/sparc32 things correct
> > again. Add the __atomic sparse address space and the store()/load()
> > accessors.
> > 
> 
> OK. I am not sure what you are referring to here, just started looking
> at the kernel again. Any pointers to this accessors thing?

lkml.kernel.org/r/20140603073613.GH11096@twins.programming.kicks-ass.net

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

end of thread, other threads:[~2014-06-06 14:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-05 22:44 regarding use of various cmpxchg* API Pranith Kumar
2014-06-06  7:29 ` Peter Zijlstra
2014-06-06 14:17   ` Pranith Kumar
2014-06-06 14:30     ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox