Re: [PATCH] Detect and warn on atomic_inc/atomic_dec wrapping around

Andrew Morton
2009-05-08T00:43:29Z

On Thu, 30 Apr 2009 19:39:50 +0530
Nikanth Karthikesan <knikanth@novell.com> wrote:

>
> Detect and warn on atomic_inc/atomic_dec overflow.
>
> Add a debug option to detect and warn when the 32-bit atomic_t overflows
> during atomic_inc and atomic_dec.
>
>
> ...
>
> --- a/include/asm-generic/atomic.h
> +++ b/include/asm-generic/atomic.h
> @@ -4,15 +4,51 @@
> * Copyright (C) 2005 Silicon Graphics, Inc.
> * Christoph Lameter
> *
> - * Allows to provide arch independent atomic definitions without the need to
> - * edit all arch specific atomic.h files.
> */
>
> +#include <linux/kernel.h>
> #include <asm/types.h>
> +#include <asm/bug.h>

We're going to have real trouble making changes like this to a
low-level header file - sparc64 results below.

> +/**
> + * atomic_inc - increment atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically increments @v by 1.
> + */
> +static inline void atomic_inc(atomic_t *v)
> +{
> +#ifdef CONFIG_ENABLE_WARN_ATOMIC_INC_WRAP
> + WARN_ONCE((atomic_read(v) > (INT_MAX / 2)),
> + KERN_ERR "atomic inc overflow!");
> +#endif
> + raw_atomic_inc(v);
> +}

Are we allowed to assume that atomic_inc==raw_atomic_inc for all
architectures which use this definition?

Do we know that atomic_read() is defined at this point?

We can avoid the problematic includes via

extern void atomic_inc_screwed_up(atomic_t *v);

static inline void atomic_inc(atomic_t *v)
{
#ifdef CONFIG_ENABLE_WARN_ATOMIC_INC_WRAP
if (atomic_read(v) > (INT_MAX / 2))
atomic_inc_screwed_up(v);
#endif
raw_atomic_inc(v);
}

In file included from /usr/src/devel/arch/sparc/include/asm/atomic_64.h:117,
from /usr/src/devel/arch/sparc/include/asm/atomic.h:4,
from include/linux/debug_locks.h:5,
from include/linux/lockdep.h:19,
from include/linux/spinlock_types.h:18,
from include/linux/spinlock.h:80,
from include/linux/seqlock.h:29,
from include/linux/time.h:8,
from include/linux/timex.h:56,
from include/linux/sched.h:54,
from arch/sparc/kernel/asm-offsets.c:13:
include/asm-generic/atomic.h:20: error: syntax error before numeric constant
include/asm-generic/atomic.h:21: warning: static declaration of 'atomic_add' follows non-static declaration
/usr/src/devel/arch/sparc/include/asm/atomic_64.h:22: warning: previous declaration of 'atomic_add' was here
include/asm-generic/atomic.h: In function `atomic_add':
include/asm-generic/atomic.h:21: error: number of arguments doesn't match prototype
/usr/src/devel/arch/sparc/include/asm/atomic_64.h:22: error: prototype declaration
include/asm-generic/atomic.h:26: error: implicit declaration of function `raw_atomic_inc'
include/asm-generic/atomic.h:26: error: `v' undeclared (first use in this function)
include/asm-generic/atomic.h:26: error: (Each undeclared identifier is reported only once
include/asm-generic/atomic.h:26: error: for each function it appears in.)
include/asm-generic/atomic.h: At top level:
include/asm-generic/atomic.h:35: error: syntax error before numeric constant
include/asm-generic/atomic.h:36: warning: static declaration of 'atomic_sub' follows non-static declaration
/usr/src/devel/arch/sparc/include/asm/atomic_64.h:24: warning: previous declaration of 'atomic_sub' was here
include/asm-generic/atomic.h: In function `atomic_sub':
include/asm-generic/atomic.h:36: error: number of arguments doesn't match prototype
/usr/src/devel/arch/sparc/include/asm/atomic_64.h:24: error: prototype declaration
include/asm-generic/atomic.h:41: error: implicit declaration of function `raw_atomic_dec'
include/asm-generic/atomic.h:41: error: `v' undeclared (first use in this function)