linux-kernel - Re: [PATCH] atomic: add atomic_inc_not_zero

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101105195101.GC15561@linux.vnet.ibm.com>
Date:	Fri, 5 Nov 2010 12:51:01 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>, Andi Kleen <andi@...stfloor.org>,
	Nick Piggin <npiggin@...nel.dk>
Subject: Re: [PATCH] atomic: add atomic_inc_not_zero_hint()

On Fri, Nov 05, 2010 at 08:20:44PM +0100, Eric Dumazet wrote:
> Le vendredi 05 novembre 2010 à 11:28 -0700, Andrew Morton a écrit :
> > But we haven't established that there _is_ duplicated code which needs
> > that treatment.
> > 
> > Scanning arch/x86/include/asm/atomic.h, perhaps ATOMIC_INIT() is a
> > candidate.  But I'm not sure that it _should_ be hoisted up - if every
> > architecture happens to do it the same way then that's just a fluke.
> > 
> > 
> 
> Not sure I understand you. I was trying to avoid recursive includes, but
> that should be protected anyway. I see a lot of code that could be
> factorized in this new header (atomic_inc_not_zero() for example)
> 
> Thanks
> 
> [PATCH v3] atomic: add atomic_inc_not_zero_hint()
> 
> Followup of perf tools session in Netfilter WorkShop 2010
> 
> In network stack we make high usage of atomic_inc_not_zero() in contexts
> we know the probable value of atomic before increment (2 for udp sockets
> for example)
> 
> Using a special version of atomic_inc_not_zero() giving this hint can
> help processor to use less bus transactions.
> 
> On x86 (MESI protocol) for example, this avoids entering Shared state,
> because "lock cmpxchg" issues an RFO (Read For Ownership)
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
> Cc: Christoph Lameter <cl@...ux-foundation.org>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Andi Kleen <andi@...stfloor.org>
> Cc: Arnaldo Carvalho de Melo <acme@...radead.org>
> Cc: David Miller <davem@...emloft.net>
> Cc: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>

Looks quite good to me!

Reviewed-by: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>

> Cc: Nick Piggin <npiggin@...nel.dk>
> ---
> V3: adds the include <asm/atomic.h>
>     if hint is null, use atomic_inc_not_zero() (Paul suggestion)
> V2: add #ifndef atomic_inc_not_zero_hint
>     kerneldoc changes
>     test that hint is not null
>     Meant to be included at end of arch/*/asm/atomic.h files
> 
> diff --git a/include/linux/atomic.h b/include/linux/atomic.h
> new file mode 100644
> index 0000000..5a7df87
> --- /dev/null
> +++ b/include/linux/atomic.h
> @@ -0,0 +1,37 @@
> +#ifndef _LINUX_ATOMIC_H
> +#define _LINUX_ATOMIC_H
> +#include <asm/atomic.h>
> +
> +/**
> + * atomic_inc_not_zero_hint - increment if not null
> + * @v: pointer of type atomic_t
> + * @hint: probable value of the atomic before the increment
> + *
> + * This version of atomic_inc_not_zero() gives a hint of probable
> + * value of the atomic. This helps processor to not read the memory
> + * before doing the atomic read/modify/write cycle, lowering
> + * number of bus transactions on some arches.
> + *
> + * Returns: 0 if increment was not done, 1 otherwise.
> + */
> +#ifndef atomic_inc_not_zero_hint
> +static inline int atomic_inc_not_zero_hint(atomic_t *v, int hint)
> +{
> +	int val, c = hint;
> +
> +	/* sanity test, should be removed by compiler if hint is a constant */
> +	if (!hint)
> +		return atomic_inc_not_zero(v);
> +
> + 	do {
> +		val = atomic_cmpxchg(v, c, c + 1);
> +		if (val == c)
> +			return 1;
> +		c = val;
> +	} while (c);
> +
> +	return 0;
> +}
> +#endif
> +
> +#endif /* _LINUX_ATOMIC_H */
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/