[PATCH] atomic: generalize atomic_add_unless_return

[PATCH] atomic: generalize atomic_add_unless_return

[Arun Sharma]

commit 686a7e3 added this primitive to get something going
for the stable branch.

Move the primitive to <linux/atomic.h>

Signed-off-by: Arun Sharma <asharma@fb.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: David Miller <davem@davemloft.net>
Cc: Andrew Morton <akpm@osdl.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
LKML-Reference: <20110531105019.GF24172@elte.hu>
---
 include/linux/atomic.h |   14 ++++++++++++++
 net/ipv4/inetpeer.c    |   17 ++---------------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index fc31190..4e2c272 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -17,6 +17,20 @@ static inline int atomic_add_unless(atomic_t *v, int a, int u)
 }
 
 /**
+ * atomic_add_unless - add unless the number is already a given value
+ * @v: pointer of type atomic_t
+ * @a: the amount to add to v...
+ * @u: ...unless v is equal to u.
+ *
+ * Atomically adds @a to @v, so long as @v was not already @u.
+ * Returns the old value of @v + @a.
+ */
+static inline int atomic_add_unless_return(atomic_t *v, int a, int u)
+{
+	return __atomic_add_unless(v, a, u) + a;
+}
+
+/**
  * atomic_inc_not_zero - increment unless the number is zero
  * @v: pointer of type atomic_t
  *
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index ce616d9..d4190ae 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -203,20 +203,6 @@ static int addr_compare(const struct inetpeer_addr *a,
 	u;							\
 })
 
-static bool atomic_add_unless_return(atomic_t *ptr, int a, int u, int *newv)
-{
-	int cur, old = atomic_read(ptr);
-
-	while (old != u) {
-		*newv = old + a;
-		cur = atomic_cmpxchg(ptr, old, *newv);
-		if (cur == old)
-			return true;
-		old = cur;
-	}
-	return false;
-}
-
 /*
  * Called with rcu_read_lock()
  * Because we hold no lock against a writer, its quite possible we fall
@@ -239,7 +225,8 @@ static struct inet_peer *lookup_rcu(const struct inetpeer_addr *daddr,
 			 * distinction between an unused entry (refcnt=0) and
 			 * a freed one.
 			 */
-			if (!atomic_add_unless_return(&u->refcnt, 1, -1, newrefcnt))
+			*newrefcnt = atomic_add_unless_return(&u->refcnt, 1, -1);
+			if (*newrefcnt == 0)
 				u = NULL;
 			return u;
 		}
-- 
1.7.4

Re: [PATCH] atomic: generalize atomic_add_unless_return

[Mike Frysinger]

On Tue, May 31, 2011 at 20:57, Arun Sharma wrote:
> commit 686a7e3 added this primitive to get something going
> for the stable branch.
>
> Move the primitive to <linux/atomic.h>

this changelog is confusing.  please replace the pronouns with proper nouns.

> --- a/include/linux/atomic.h
> +++ b/include/linux/atomic.h
>  /**
> + * atomic_add_unless - add unless the number is already a given value
> + * @v: pointer of type atomic_t
> + * @a: the amount to add to v...
> + * @u: ...unless v is equal to u.
> + *
> + * Atomically adds @a to @v, so long as @v was not already @u.
> + * Returns the old value of @v + @a.
> + */
> +static inline int atomic_add_unless_return(atomic_t *v, int a, int u)

the name of the func in the comment does not match the actual name of the func

also, the semantics here appear to deviate from the other atomic
funcs.  the existing xxx_return funcs return the new result, not the
old.
-mike

Re: [PATCH] atomic: generalize atomic_add_unless_return

[Eric Dumazet]

Le dimanche 05 juin 2011 à 00:09 -0400, Mike Frysinger a écrit :
> On Tue, May 31, 2011 at 20:57, Arun Sharma wrote:

> > --- a/include/linux/atomic.h
> > +++ b/include/linux/atomic.h
> >  /**
> > + * atomic_add_unless - add unless the number is already a given value
> > + * @v: pointer of type atomic_t
> > + * @a: the amount to add to v...
> > + * @u: ...unless v is equal to u.
> > + *
> > + * Atomically adds @a to @v, so long as @v was not already @u.
> > + * Returns the old value of @v + @a.
> > + */
> > +static inline int atomic_add_unless_return(atomic_t *v, int a, int u)
> 
> the name of the func in the comment does not match the actual name of the func
> 
> also, the semantics here appear to deviate from the other atomic
> funcs.  the existing xxx_return funcs return the new result, not the
> old.
> -mike

Arun comment is wrong, this function does return the new value of the
atomic_t after our operation, not the old ;)

Re: [PATCH] atomic: generalize atomic_add_unless_return

[Arun Sharma]

On 6/5/11 12:35 AM, Eric Dumazet wrote:

>>> + * Returns the old value of @v + @a.
>>> + */
>>> +static inline int atomic_add_unless_return(atomic_t *v, int a, int u)
>>  [...]
>>
>> also, the semantics here appear to deviate from the other atomic
>> funcs.  the existing xxx_return funcs return the new result, not the
>> old.
>> -mike
>
> Arun comment is wrong, this function does return the new value of the
> atomic_t after our operation, not the old ;)

The comment is technically correct:

old value of @v + @a = new value of @v.

I'll try to make it less confusing :)

Mike: Thanks for the review. I'll fix up the commit log and the func 
name and post a new patch.

  -Arun