Sync operation in atomic_add_return()

Sync operation in atomic_add_return()

[Gideon Stupp (gstupp)]

Hi,
I am trying to figure out why there is a sync operation in
linux/include/asm-mips/atomic.h:atomic_add_return(). 
I believe it was added in the linux-2.4.19 patch, but can't trace the
reason. Can anyone help?

Thanks, Gideon.

Sync operation in atomic_add_return()

[Gideon Stupp (gstupp)]

Hi,
I am trying to figure out why there is a sync operation in
linux/include/asm-mips/atomic.h:atomic_add_return(). 
I believe it was added in the linux-2.4.19 patch, but can't trace the
reason. Can anyone help?

Thanks, Gideon.

Re: Sync operation in atomic_add_return()

[Ralf Baechle]

On Mon, Nov 06, 2006 at 03:42:32PM +0100, Gideon Stupp (gstupp) wrote:

> I am trying to figure out why there is a sync operation in
> linux/include/asm-mips/atomic.h:atomic_add_return(). 
> I believe it was added in the linux-2.4.19 patch, but can't trace the
> reason. Can anyone help?

MIPS is a weakly ordered architecture.  In theory.  So those syncs are
required to ensure proper global ordering.  In practice only the Sibyte
SB1 and RM9000 CMPs are documented to be weakly ordered but I've never
actually observed a single reordering related bug on any MIPS
multiprocessor which may either mean no reordering happens in practice
or I'm a genious managed to fix all reordering related bugs before they
could strike.  I tend to assume the latter ;-)  On a uniprocessor these
syncs are definately not needed and I have a patch to remove the sync
for uniprocessor kernels and known to be strongly ordered SMPs waiting
for 2.6.20.

  Ralf

RE: Sync operation in atomic_add_return()

[Kaz Kylheku]

Gideon Stupp wrote:
> Hi,
> I am trying to figure out why there is a sync operation in
> linux/include/asm-mips/atomic.h:atomic_add_return(). 
> I believe it was added in the linux-2.4.19 patch, but can't trace the
> reason. Can anyone help?

Is it just unwarranted paranoia? There does not appear to be a need for
the sync within the atomic_add_return code itself.

But it might be that the code which calls this function needs the sync.

Without looking at any code whatsoever, here is a general hypothesis.

In what situation might you /care/ about the return value of an atomic
add?

Suppose atomic increments and decrements are being used for reference
counting. If you know that you hold the reference to an object, you can
call atomic_add to increase the reference count without caring about the
return value, and no sync is needed in that situation.

Suppose, however, that atomic_add is used to pick up a reference.
Suppose you have a pool of ``dead'' objects with reference counts of
zero, and want to recycle an object from such a pool. You might use
atomic_add_return to examine the reference counts of the objects in this
pool one by one until you get a 1 return. You might get something other
than a 1 return if racing against another processor which is tryiing to
pick up the same object.

In this situation, if you successfully get the object, you do want to do
a sync, since the object is being handed off between two processors.
Before the object was put into the pool, its fields were updated, since
it was being cleaned up. You would not want the new owner, by chance, to
observe stale values of those fields.

I.e., to put it briefly, atomic_add_return can have "acquire" semantics.

RE: Sync operation in atomic_add_return()

[Kaz Kylheku]

Gideon Stupp wrote:
> Hi,
> I am trying to figure out why there is a sync operation in
> linux/include/asm-mips/atomic.h:atomic_add_return(). 
> I believe it was added in the linux-2.4.19 patch, but can't trace the
> reason. Can anyone help?

Is it just unwarranted paranoia? There does not appear to be a need for
the sync within the atomic_add_return code itself.

But it might be that the code which calls this function needs the sync.

Without looking at any code whatsoever, here is a general hypothesis.

In what situation might you /care/ about the return value of an atomic
add?

Suppose atomic increments and decrements are being used for reference
counting. If you know that you hold the reference to an object, you can
call atomic_add to increase the reference count without caring about the
return value, and no sync is needed in that situation.

Suppose, however, that atomic_add is used to pick up a reference.
Suppose you have a pool of ``dead'' objects with reference counts of
zero, and want to recycle an object from such a pool. You might use
atomic_add_return to examine the reference counts of the objects in this
pool one by one until you get a 1 return. You might get something other
than a 1 return if racing against another processor which is tryiing to
pick up the same object.

In this situation, if you successfully get the object, you do want to do
a sync, since the object is being handed off between two processors.
Before the object was put into the pool, its fields were updated, since
it was being cleaned up. You would not want the new owner, by chance, to
observe stale values of those fields.

I.e., to put it briefly, atomic_add_return can have "acquire" semantics.

Re: Sync operation in atomic_add_return()

[Ralf Baechle]

On Mon, Nov 06, 2006 at 10:17:53AM -0800, Kaz Kylheku wrote:

> > Hi,
> > I am trying to figure out why there is a sync operation in
> > linux/include/asm-mips/atomic.h:atomic_add_return(). 
> > I believe it was added in the linux-2.4.19 patch, but can't trace the
> > reason. Can anyone help?
> 
> Is it just unwarranted paranoia? There does not appear to be a need for
> the sync within the atomic_add_return code itself.

atomic_*_return() are used as synchronization points so must imply a
memory barrier on MP.

> But it might be that the code which calls this function needs the sync.
> 
> Without looking at any code whatsoever, here is a general hypothesis.
> 
> In what situation might you /care/ about the return value of an atomic
> add?
> 
> Suppose atomic increments and decrements are being used for reference
> counting. If you know that you hold the reference to an object, you can
> call atomic_add to increase the reference count without caring about the
> return value, and no sync is needed in that situation.

For example the networking code does basically:

static inline void sock_put(struct sock *sk)
{
	if (atomic_add_and_test(-11, &sk->sk_refcnt) == 0)
		sk_free(sk);
}

> Suppose, however, that atomic_add is used to pick up a reference.
> Suppose you have a pool of ``dead'' objects with reference counts of
> zero, and want to recycle an object from such a pool. You might use
> atomic_add_return to examine the reference counts of the objects in this
> pool one by one until you get a 1 return. You might get something other
> than a 1 return if racing against another processor which is tryiing to
> pick up the same object.
> 
> In this situation, if you successfully get the object, you do want to do
> a sync, since the object is being handed off between two processors.
> Before the object was put into the pool, its fields were updated, since
> it was being cleaned up. You would not want the new owner, by chance, to
> observe stale values of those fields.
> 
> I.e., to put it briefly, atomic_add_return can have "acquire" semantics.

Correct - and depending on its use it may also have release semantics.
This applies to all atomic_*_return() functions not just atomic_add_return.

  Ralf

RE: Sync operation in atomic_add_return()

[Gideon Stupp (gstupp)]

> -----Original Message-----
> From: linux-mips-bounce@linux-mips.org 
> [mailto:linux-mips-bounce@linux-mips.org] On Behalf Of Kaz Kylheku
> Sent: Monday, November 06, 2006 8:18 PM
> To: linux-mips@linux-mips.org
> Subject: RE: Sync operation in atomic_add_return()
> 
> Gideon Stupp wrote:
> > Hi,
> > I am trying to figure out why there is a sync operation in 
> > linux/include/asm-mips/atomic.h:atomic_add_return().
> > I believe it was added in the linux-2.4.19 patch, but can't 
> trace the 
> > reason. Can anyone help?
> 
> Is it just unwarranted paranoia? There does not appear to be 
> a need for the sync within the atomic_add_return code itself.
> 
> But it might be that the code which calls this function needs 
> the sync.
> 
> Without looking at any code whatsoever, here is a general hypothesis.
> 
> In what situation might you /care/ about the return value of 
> an atomic add?
> 
> Suppose atomic increments and decrements are being used for 
> reference counting. If you know that you hold the reference 
> to an object, you can call atomic_add to increase the 
> reference count without caring about the return value, and no 
> sync is needed in that situation.
> 
> Suppose, however, that atomic_add is used to pick up a reference.
> Suppose you have a pool of ``dead'' objects with reference 
> counts of zero, and want to recycle an object from such a 
> pool. You might use atomic_add_return to examine the 
> reference counts of the objects in this pool one by one until 
> you get a 1 return. You might get something other than a 1 
> return if racing against another processor which is tryiing 
> to pick up the same object.
> 
> In this situation, if you successfully get the object, you do 
> want to do a sync, since the object is being handed off 
> between two processors.
> Before the object was put into the pool, its fields were 
> updated, since it was being cleaned up. You would not want 
> the new owner, by chance, to observe stale values of those fields.
> 
> I.e., to put it briefly, atomic_add_return can have "acquire" 
> semantics.
> 

Thanks for the reply.  I also checked the Alpha implementation ( the
only other architecture I know of with non serializing atomic operations
) and indeed there is an explicit smp_mb() in atomic_add_return() and
nowhere else.  So I guess this is the convention.

Gideon.

RE: Sync operation in atomic_add_return()

[Gideon Stupp (gstupp)]

> -----Original Message-----
> From: linux-mips-bounce@linux-mips.org 
> [mailto:linux-mips-bounce@linux-mips.org] On Behalf Of Kaz Kylheku
> Sent: Monday, November 06, 2006 8:18 PM
> To: linux-mips@linux-mips.org
> Subject: RE: Sync operation in atomic_add_return()
> 
> Gideon Stupp wrote:
> > Hi,
> > I am trying to figure out why there is a sync operation in 
> > linux/include/asm-mips/atomic.h:atomic_add_return().
> > I believe it was added in the linux-2.4.19 patch, but can't 
> trace the 
> > reason. Can anyone help?
> 
> Is it just unwarranted paranoia? There does not appear to be 
> a need for the sync within the atomic_add_return code itself.
> 
> But it might be that the code which calls this function needs 
> the sync.
> 
> Without looking at any code whatsoever, here is a general hypothesis.
> 
> In what situation might you /care/ about the return value of 
> an atomic add?
> 
> Suppose atomic increments and decrements are being used for 
> reference counting. If you know that you hold the reference 
> to an object, you can call atomic_add to increase the 
> reference count without caring about the return value, and no 
> sync is needed in that situation.
> 
> Suppose, however, that atomic_add is used to pick up a reference.
> Suppose you have a pool of ``dead'' objects with reference 
> counts of zero, and want to recycle an object from such a 
> pool. You might use atomic_add_return to examine the 
> reference counts of the objects in this pool one by one until 
> you get a 1 return. You might get something other than a 1 
> return if racing against another processor which is tryiing 
> to pick up the same object.
> 
> In this situation, if you successfully get the object, you do 
> want to do a sync, since the object is being handed off 
> between two processors.
> Before the object was put into the pool, its fields were 
> updated, since it was being cleaned up. You would not want 
> the new owner, by chance, to observe stale values of those fields.
> 
> I.e., to put it briefly, atomic_add_return can have "acquire" 
> semantics.
> 

Thanks for the reply.  I also checked the Alpha implementation ( the
only other architecture I know of with non serializing atomic operations
) and indeed there is an explicit smp_mb() in atomic_add_return() and
nowhere else.  So I guess this is the convention.

Gideon.