All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arun Sharma <asharma@fb.com>
To: Mike Frysinger <vapier.adi@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Ingo Molnar <mingo@elte.hu>, David Miller <davem@davemloft.net>
Subject: Re: [PATCH] atomic: cleanup asm-generic atomic*.h inclusion
Date: Fri, 17 Jun 2011 11:42:09 -0700	[thread overview]
Message-ID: <4DFBA001.4010500@fb.com> (raw)
In-Reply-To: <BANLkTim5zEo=Rn2sqUExL-8n=p4inLEpuQ@mail.gmail.com>

On 6/17/11 11:06 AM, Mike Frysinger wrote:

> fixes one thing while breaking another.  linux/atomic.h includes
> asm/atomic.h which includes asm-generic/atomic.h which includes
> asm-generic/atomic-long.h which needs atomic_add_unless(), but that
> isnt provided until after the asm/atomic.h include in linux/atomic.h.
>
> but linux/atomic.h needs asm/atomic.h before atomic_add_unless()
> because it relies on the new __atomic_add_unless().
>

Right. I think we need to get rid of that include as well:

--- a/include/asm-generic/atomic.h
+++ b/include/asm-generic/atomic.h
@@ -156,7 +145,5 @@ static inline void atomic_clear_mask(unsigned long 
mask, unsigned long *addr)
  #define smp_mb__before_atomic_inc()    barrier()
  #define smp_mb__after_atomic_inc()     barrier()

-#include <asm-generic/atomic-long.h>
-
  #endif /* __KERNEL__ */
  #endif /* __ASM_GENERIC_ATOMIC_H */

since <linux/atomic.h> includes it directly now.

> having linux/atomic.h and asm-generic/atomic.h just strikes me as
> wrong.  the point of asm-generic is to unify things, but now we have
> two places to unify things without 0 indication as to which is for
> which ?  i'm wondering if we shouldnt convert all arches to
> asm-generic/atomic.h and then add your new logic there and just skip
> this whole linux/atomic.h mess.

I believe the logic is:

<asm-generic/atomic.h> shared code for simple archs that don't want to 
define their own primitives. Only used by 4 archs.

<linux/atomic.h> a header file that's usable in machine independent 
kernel code.

<linux/atomic.h> seems to be relatively recent, introduced by:

commit 3f9d35b9514da6757ca98831372518f9eeb71b33
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date:   Thu Nov 11 14:05:08 2010 -0800

     atomic: add atomic_inc_not_zero_hint()

<asm-generic/atomic.h> came from:

commit d3cb487149bd706aa6aeb02042332a450978dc1c
Author: Christoph Lameter <clameter@engr.sgi.com>
Date:   Fri Jan 6 00:11:20 2006 -0800

     [PATCH] atomic_long_t & include/asm-generic/atomic.h V2

My guess is that if <linux/atomic.h> existed in 2006, people would've 
added shared code over there. Also, the code in <asm-generic/atomic.h> 
is not truly generic:

smp_mb__before_atomic_dec() for eg is implemented differently for some 
archs.

  -Arun

  reply	other threads:[~2011-06-17 18:41 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-06 17:27 [PATCH 1/3] atomic: use <linux/atomic.h> Arun Sharma
2011-06-06 17:27 ` [PATCH 2/3] atomic: move atomic_add_unless to generic code Arun Sharma
2011-06-06 20:56   ` Eric Dumazet
2011-06-06 17:27 ` [PATCH 3/3] atomic: generalize atomic_add_unless_return Arun Sharma
2011-06-06 20:58   ` Eric Dumazet
2011-06-09 18:01     ` Eric Dumazet
2011-06-06 20:55 ` [PATCH 1/3] atomic: use <linux/atomic.h> Eric Dumazet
2011-06-06 21:06   ` Arun Sharma
2011-06-15 23:31 ` Andrew Morton
2011-06-16  0:57   ` Arun Sharma
2011-06-16  1:29   ` [PATCH] atomic: cleanup asm-generic atomic*.h inclusion Arun Sharma
2011-06-17  6:36     ` Mike Frysinger
2011-06-17 17:11       ` Arun Sharma
2011-06-17 17:28         ` Mike Frysinger
2011-06-17 17:49           ` Arun Sharma
2011-06-17 18:06             ` Mike Frysinger
2011-06-17 18:42               ` Arun Sharma [this message]
2011-06-17 19:07                 ` Mike Frysinger
2011-06-17 20:05                   ` Arun Sharma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DFBA001.4010500@fb.com \
    --to=asharma@fb.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=vapier.adi@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.