Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1243784 > unrolled thread

Re: [RFC v2 5/7] powerpc: atomic: Implement cmpxchg{,64}_* and atomic{,64}_cmpxchg_* variants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC v2 5/7] powerpc: atomic: Implement cmpxchg{,64}_* and  atomic{,64}_cmpxchg_* variants Boqun Feng <boqun.feng@gmail.com> - 2015-10-10 04:00 +0200
    Re: [RFC v2 5/7] powerpc: atomic: Implement cmpxchg{,64}_* and  atomic{,64}_cmpxchg_* variants Boqun Feng <boqun.feng@gmail.com> - 2015-10-11 12:30 +0200
      Re: [RFC v2 5/7] powerpc: atomic: Implement cmpxchg{,64}_* and  atomic{,64}_cmpxchg_* variants Peter Zijlstra <peterz@infradead.org> - 2015-10-12 08:50 +0200
        Re: [RFC v2 5/7] powerpc: atomic: Implement cmpxchg{,64}_* and  atomic{,64}_cmpxchg_* variants Boqun Feng <boqun.feng@gmail.com> - 2015-10-12 09:10 +0200

#1243784 — Re: [RFC v2 5/7] powerpc: atomic: Implement cmpxchg{,64}_* and atomic{,64}_cmpxchg_* variants

[Multipart message — attachments visible in raw view] — view raw

Hi Peter,

Sorry for replying late.

On Thu, Oct 01, 2015 at 02:27:16PM +0200, Peter Zijlstra wrote:
> On Wed, Sep 16, 2015 at 11:49:33PM +0800, Boqun Feng wrote:
> > Unlike other atomic operation variants, cmpxchg{,64}_acquire and
> > atomic{,64}_cmpxchg_acquire don't have acquire semantics if the cmp part
> > fails, so we need to implement these using assembly.
> 
> I think that is actually expected and documented. That is, a cmpxchg
> only implies barriers on success. See:
> 
>   ed2de9f74ecb ("locking/Documentation: Clarify failed cmpxchg() memory ordering semantics")

I probably didn't make myself clear here, my point is that if we use
__atomic_op_acquire() to built *_cmpchg_acquire(For ARM and PowerPC),
the barrier will be implied _unconditionally_, meaning no matter cmp
fails or not, there will be a barrier after the cmpxchg operation.
Therefore we have to use assembly to implement the operations right now.

Regards,
Boqun

[toc] | [next] | [standalone]

#1244093

[Multipart message — attachments visible in raw view] — view raw

On Sat, Oct 10, 2015 at 09:58:05AM +0800, Boqun Feng wrote:
> Hi Peter,
> 
> Sorry for replying late.
> 
> On Thu, Oct 01, 2015 at 02:27:16PM +0200, Peter Zijlstra wrote:
> > On Wed, Sep 16, 2015 at 11:49:33PM +0800, Boqun Feng wrote:
> > > Unlike other atomic operation variants, cmpxchg{,64}_acquire and
> > > atomic{,64}_cmpxchg_acquire don't have acquire semantics if the cmp part
> > > fails, so we need to implement these using assembly.
> > 
> > I think that is actually expected and documented. That is, a cmpxchg
> > only implies barriers on success. See:
> > 
> >   ed2de9f74ecb ("locking/Documentation: Clarify failed cmpxchg() memory ordering semantics")
> 
> I probably didn't make myself clear here, my point is that if we use
> __atomic_op_acquire() to built *_cmpchg_acquire(For ARM and PowerPC),
> the barrier will be implied _unconditionally_, meaning no matter cmp
> fails or not, there will be a barrier after the cmpxchg operation.
> Therefore we have to use assembly to implement the operations right now.
> 

Or let me try another way to explain this. What I wanted to say here is
that unlike the implementation of xchg family, which needs only to
implement _relaxed version and *remove* the fully ordered version, the
implementation of cmpxchg family needs to *remain* the fully ordered
version and implement the _acquire version in assembly. Because if we
use __atomic_op_*(), the barriers in the cmpxchg family will be implied
*unconditionally*, for example:

cmpxchg() on PPC will be(built by my version of __atomic_op_fence()):

	smp_lwsync();
	cmpxchg_relaxed(...);
	smp_mb__after_atomic(); // a full barrier regardless of success
				// or failure.

In order to have a conditional barrier, we need a way to jump out of a
ll/sc loop, which could only(?) be done by assembly code.

My commit log surely failed to explain this clearly, I will modifiy that
in next series. In the meanwhile, looking forwards to suggestion on the
implementation of cmpxchg familiy ;-)

BTW, Will, could you please check whether the barriers in cmpxchg family
are unconditional or not in the current implementation of ARM? IIUC,
they are currently unconditional, right?

Regards,
Boqun

[toc] | [prev] | [next] | [standalone]

#1244408

On Sun, Oct 11, 2015 at 06:25:20PM +0800, Boqun Feng wrote:
> On Sat, Oct 10, 2015 at 09:58:05AM +0800, Boqun Feng wrote:
> > Hi Peter,
> > 
> > Sorry for replying late.
> > 
> > On Thu, Oct 01, 2015 at 02:27:16PM +0200, Peter Zijlstra wrote:
> > > On Wed, Sep 16, 2015 at 11:49:33PM +0800, Boqun Feng wrote:
> > > > Unlike other atomic operation variants, cmpxchg{,64}_acquire and
> > > > atomic{,64}_cmpxchg_acquire don't have acquire semantics if the cmp part
> > > > fails, so we need to implement these using assembly.
> > > 
> > > I think that is actually expected and documented. That is, a cmpxchg
> > > only implies barriers on success. See:
> > > 
> > >   ed2de9f74ecb ("locking/Documentation: Clarify failed cmpxchg() memory ordering semantics")
> > 
> > I probably didn't make myself clear here, my point is that if we use
> > __atomic_op_acquire() to built *_cmpchg_acquire(For ARM and PowerPC),
> > the barrier will be implied _unconditionally_, meaning no matter cmp
> > fails or not, there will be a barrier after the cmpxchg operation.
> > Therefore we have to use assembly to implement the operations right now.

See later, but no, you don't _have_ to.

> Or let me try another way to explain this. What I wanted to say here is
> that unlike the implementation of xchg family, which needs only to
> implement _relaxed version and *remove* the fully ordered version, the
> implementation of cmpxchg family needs to *remain* the fully ordered
> version and implement the _acquire version in assembly. Because if we
> use __atomic_op_*(), the barriers in the cmpxchg family will be implied
> *unconditionally*, for example:

So the point that confused me, and which is still valid for the above,
is your use of 'need'.

You don't need to omit the barrier at all. Its perfectly valid to issue
too many barriers (pointless and a waste of time, yes; incorrect, no).

So what you want to say is: "Optimize cmpxchg_acquire() to avoid
superfluous barrier".
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244426

[Multipart message — attachments visible in raw view] — view raw

On Mon, Oct 12, 2015 at 08:46:21AM +0200, Peter Zijlstra wrote:
> On Sun, Oct 11, 2015 at 06:25:20PM +0800, Boqun Feng wrote:
> > On Sat, Oct 10, 2015 at 09:58:05AM +0800, Boqun Feng wrote:
> > > Hi Peter,
> > > 
> > > Sorry for replying late.
> > > 
> > > On Thu, Oct 01, 2015 at 02:27:16PM +0200, Peter Zijlstra wrote:
> > > > On Wed, Sep 16, 2015 at 11:49:33PM +0800, Boqun Feng wrote:
> > > > > Unlike other atomic operation variants, cmpxchg{,64}_acquire and
> > > > > atomic{,64}_cmpxchg_acquire don't have acquire semantics if the cmp part
> > > > > fails, so we need to implement these using assembly.
> > > > 
> > > > I think that is actually expected and documented. That is, a cmpxchg
> > > > only implies barriers on success. See:
> > > > 
> > > >   ed2de9f74ecb ("locking/Documentation: Clarify failed cmpxchg() memory ordering semantics")
> > > 
> > > I probably didn't make myself clear here, my point is that if we use
> > > __atomic_op_acquire() to built *_cmpchg_acquire(For ARM and PowerPC),
> > > the barrier will be implied _unconditionally_, meaning no matter cmp
> > > fails or not, there will be a barrier after the cmpxchg operation.
> > > Therefore we have to use assembly to implement the operations right now.
> 
> See later, but no, you don't _have_ to.
> 
> > Or let me try another way to explain this. What I wanted to say here is
> > that unlike the implementation of xchg family, which needs only to
> > implement _relaxed version and *remove* the fully ordered version, the
> > implementation of cmpxchg family needs to *remain* the fully ordered
> > version and implement the _acquire version in assembly. Because if we
> > use __atomic_op_*(), the barriers in the cmpxchg family will be implied
> > *unconditionally*, for example:
> 
> So the point that confused me, and which is still valid for the above,
> is your use of 'need'.
> 

Indeed, my apologies for confusing more..

> You don't need to omit the barrier at all. Its perfectly valid to issue
> too many barriers (pointless and a waste of time, yes; incorrect, no).
> 

Agreed.

> So what you want to say is: "Optimize cmpxchg_acquire() to avoid
> superfluous barrier".

Yes. I would like to implement a cmpxchg_acquire that really has no
barrier if cmp failed, which became my 'need' for the implementation.
But you are right, we don't have to omit the barrier. I will modify the
commit log to explain this clear in the next version. Thank you!

Regards,
Boqun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web