Re: [PATCH 1/23] document preferred use of volatile with atomic_t
But barriers force a flush of *everything* in scope, Nonsense, whatever "flush" is supposed to mean here. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. Puh-lease. I DO NOT DISTRUST THE COMPILER, I just don't assume it will do whatever I would like it to do without telling it. It's a machine you know, and it is very well documented. (And most barriers don't (need to) use volatile). Implementing the atomic ops in asm loses exactly *no* semantics, and it doesn't add restrictions either; it does allow you however to access an atomic_t with normal loads/stores independently, if you so choose. The only valid issue brought up so far is Russell King's comment that GCC cannot schedule the machine instructions in and around an asm() perfectly, it doesn't look inside the asm() after all; but the situation isn't quite as sever as he suggests (GCC _does_ know you are performing loads/stores, etc.); more about that later, when I've finished researching the current state of that stuff. Segher - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
But barriers force a flush of *everything* in scope, Nonsense, whatever flush is supposed to mean here. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. Puh-lease. I DO NOT DISTRUST THE COMPILER, I just don't assume it will do whatever I would like it to do without telling it. It's a machine you know, and it is very well documented. (And most barriers don't (need to) use volatile). Implementing the atomic ops in asm loses exactly *no* semantics, and it doesn't add restrictions either; it does allow you however to access an atomic_t with normal loads/stores independently, if you so choose. The only valid issue brought up so far is Russell King's comment that GCC cannot schedule the machine instructions in and around an asm() perfectly, it doesn't look inside the asm() after all; but the situation isn't quite as sever as he suggests (GCC _does_ know you are performing loads/stores, etc.); more about that later, when I've finished researching the current state of that stuff. Segher - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Tue, Aug 14, 2007 at 03:56:51PM -0700, Christoph Lameter wrote: > On Tue, 14 Aug 2007, Chris Snook wrote: > > > > volatile means that there is some vague notion of "read it now". But that > > > really does not exist. Instead we control visibility via barriers > > > (smp_wmb, > > > smp_rmb). Would it not be best to not have volatile at all in atomic > > > operations and let the barriers do the work? > > > > From my reply in the other thread... > > > > But barriers force a flush of *everything* in scope, which we generally > > don't > > want. On the other hand, we pretty much always want to flush atomic_* > > operations. One way or another, we should be restricting the volatile > > behavior to the thing that needs it. On most architectures, this patch set > > just moves that from the declaration, where it is considered harmful, to the > > use, where it is considered an occasional necessary evil. > > > > If you really, *really* distrust the compiler that much, you shouldn't be > > using barrier, since that uses volatile under the hood too. You should just > > go ahead and implement the atomic operations in assembler, like Segher > > Boessenkool did for powerpc in response to my previous patchset. > > >From my reply on the other thread: > > Maybe we need two read functions? One volatile, one not? > > The atomic_read()s that I have in slub really do not care about when the > variables are read. And if volatile creates overhead then I rather not have > it. The overhead due to volatile access is -way- small. Not like barrier(), which can flush out a fair fraction of the machine registers. Thanx, Paul - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
Christoph Lameter wrote: On Tue, 14 Aug 2007, Chris Snook wrote: volatile means that there is some vague notion of "read it now". But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? From my reply in the other thread... But barriers force a flush of *everything* in scope, which we generally don't want. On the other hand, we pretty much always want to flush atomic_* operations. One way or another, we should be restricting the volatile behavior to the thing that needs it. On most architectures, this patch set just moves that from the declaration, where it is considered harmful, to the use, where it is considered an occasional necessary evil. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. From my reply on the other thread: Maybe we need two read functions? One volatile, one not? If we're going to do this, and I don't think we need to, I'd prefer that atomic_read() be volatile, and something like atomic_read_opt() be non-volatile, to discourage premature optimization. The atomic_read()s that I have in slub really do not care about when the variables are read. And if volatile creates overhead then I rather not have it. A single volatile access is no more expensive than a non-volatile access. It's when you have dependencies that you start to see overhead. If you're doing a bunch of atomic operations on the same atomic_t in quick succession, then you will see some overhead. Of course, if you're doing that, I think you have a design problem. On modern, register-rich CPUs with cache latencies of a couple clock cycles, volatile generally isn't as much of a performance hit as it used to be. I think that going out of your way to avoid it would be premature optimization on modern hardware. -- Chris - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Tue, 14 Aug 2007, Chris Snook wrote: > > volatile means that there is some vague notion of "read it now". But that > > really does not exist. Instead we control visibility via barriers (smp_wmb, > > smp_rmb). Would it not be best to not have volatile at all in atomic > > operations and let the barriers do the work? > > From my reply in the other thread... > > But barriers force a flush of *everything* in scope, which we generally don't > want. On the other hand, we pretty much always want to flush atomic_* > operations. One way or another, we should be restricting the volatile > behavior to the thing that needs it. On most architectures, this patch set > just moves that from the declaration, where it is considered harmful, to the > use, where it is considered an occasional necessary evil. > > If you really, *really* distrust the compiler that much, you shouldn't be > using barrier, since that uses volatile under the hood too. You should just > go ahead and implement the atomic operations in assembler, like Segher > Boessenkool did for powerpc in response to my previous patchset. >From my reply on the other thread: Maybe we need two read functions? One volatile, one not? The atomic_read()s that I have in slub really do not care about when the variables are read. And if volatile creates overhead then I rather not have it. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
Christoph Lameter wrote: On Mon, 13 Aug 2007, Chris Snook wrote: @@ -38,7 +45,7 @@ Next, we have: - #define atomic_read(v) ((v)->counter) + #define atomic_read(v) (*(volatile int *)&(v)->counter) which simply reads the current value of the counter. volatile means that there is some vague notion of "read it now". But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? From my reply in the other thread... But barriers force a flush of *everything* in scope, which we generally don't want. On the other hand, we pretty much always want to flush atomic_* operations. One way or another, we should be restricting the volatile behavior to the thing that needs it. On most architectures, this patch set just moves that from the declaration, where it is considered harmful, to the use, where it is considered an occasional necessary evil. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. -- Chris - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Mon, 13 Aug 2007, Chris Snook wrote: > @@ -38,7 +45,7 @@ > > Next, we have: > > - #define atomic_read(v) ((v)->counter) > + #define atomic_read(v) (*(volatile int *)&(v)->counter) > > which simply reads the current value of the counter. volatile means that there is some vague notion of "read it now". But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Mon, 13 Aug 2007, Chris Snook wrote: @@ -38,7 +45,7 @@ Next, we have: - #define atomic_read(v) ((v)-counter) + #define atomic_read(v) (*(volatile int *)(v)-counter) which simply reads the current value of the counter. volatile means that there is some vague notion of read it now. But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
Christoph Lameter wrote: On Mon, 13 Aug 2007, Chris Snook wrote: @@ -38,7 +45,7 @@ Next, we have: - #define atomic_read(v) ((v)-counter) + #define atomic_read(v) (*(volatile int *)(v)-counter) which simply reads the current value of the counter. volatile means that there is some vague notion of read it now. But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? From my reply in the other thread... But barriers force a flush of *everything* in scope, which we generally don't want. On the other hand, we pretty much always want to flush atomic_* operations. One way or another, we should be restricting the volatile behavior to the thing that needs it. On most architectures, this patch set just moves that from the declaration, where it is considered harmful, to the use, where it is considered an occasional necessary evil. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. -- Chris - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Tue, 14 Aug 2007, Chris Snook wrote: volatile means that there is some vague notion of read it now. But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? From my reply in the other thread... But barriers force a flush of *everything* in scope, which we generally don't want. On the other hand, we pretty much always want to flush atomic_* operations. One way or another, we should be restricting the volatile behavior to the thing that needs it. On most architectures, this patch set just moves that from the declaration, where it is considered harmful, to the use, where it is considered an occasional necessary evil. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. From my reply on the other thread: Maybe we need two read functions? One volatile, one not? The atomic_read()s that I have in slub really do not care about when the variables are read. And if volatile creates overhead then I rather not have it. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
Christoph Lameter wrote: On Tue, 14 Aug 2007, Chris Snook wrote: volatile means that there is some vague notion of read it now. But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? From my reply in the other thread... But barriers force a flush of *everything* in scope, which we generally don't want. On the other hand, we pretty much always want to flush atomic_* operations. One way or another, we should be restricting the volatile behavior to the thing that needs it. On most architectures, this patch set just moves that from the declaration, where it is considered harmful, to the use, where it is considered an occasional necessary evil. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. From my reply on the other thread: Maybe we need two read functions? One volatile, one not? If we're going to do this, and I don't think we need to, I'd prefer that atomic_read() be volatile, and something like atomic_read_opt() be non-volatile, to discourage premature optimization. The atomic_read()s that I have in slub really do not care about when the variables are read. And if volatile creates overhead then I rather not have it. A single volatile access is no more expensive than a non-volatile access. It's when you have dependencies that you start to see overhead. If you're doing a bunch of atomic operations on the same atomic_t in quick succession, then you will see some overhead. Of course, if you're doing that, I think you have a design problem. On modern, register-rich CPUs with cache latencies of a couple clock cycles, volatile generally isn't as much of a performance hit as it used to be. I think that going out of your way to avoid it would be premature optimization on modern hardware. -- Chris - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Tue, Aug 14, 2007 at 03:56:51PM -0700, Christoph Lameter wrote: On Tue, 14 Aug 2007, Chris Snook wrote: volatile means that there is some vague notion of read it now. But that really does not exist. Instead we control visibility via barriers (smp_wmb, smp_rmb). Would it not be best to not have volatile at all in atomic operations and let the barriers do the work? From my reply in the other thread... But barriers force a flush of *everything* in scope, which we generally don't want. On the other hand, we pretty much always want to flush atomic_* operations. One way or another, we should be restricting the volatile behavior to the thing that needs it. On most architectures, this patch set just moves that from the declaration, where it is considered harmful, to the use, where it is considered an occasional necessary evil. If you really, *really* distrust the compiler that much, you shouldn't be using barrier, since that uses volatile under the hood too. You should just go ahead and implement the atomic operations in assembler, like Segher Boessenkool did for powerpc in response to my previous patchset. From my reply on the other thread: Maybe we need two read functions? One volatile, one not? The atomic_read()s that I have in slub really do not care about when the variables are read. And if volatile creates overhead then I rather not have it. The overhead due to volatile access is -way- small. Not like barrier(), which can flush out a fair fraction of the machine registers. Thanx, Paul - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Mon, Aug 13, 2007 at 07:04:15AM -0400, Chris Snook wrote:
> From: Chris Snook <[EMAIL PROTECTED]>
>
> Document proper use of volatile for atomic_t operations.
Looks good, as did a once-over on the arch-specific files. Good stuff!!!
Acked-by: Paul E. McKenney <[EMAIL PROTECTED]>
> Signed-off-by: Chris Snook <[EMAIL PROTECTED]>
>
> --- linux-2.6.23-rc3-orig/Documentation/atomic_ops.txt2007-07-08
> 19:32:17.0 -0400
> +++ linux-2.6.23-rc3/Documentation/atomic_ops.txt 2007-08-13
> 03:36:43.0 -0400
> @@ -12,13 +12,20 @@
> C integer type will fail. Something like the following should
> suffice:
>
> - typedef struct { volatile int counter; } atomic_t;
> + typedef struct { int counter; } atomic_t;
> +
> + Historically, counter has been declared as a volatile int. This
> +is now discouraged in favor of explicitly casting it as volatile where
> +volatile behavior is required. Most architectures will only require such
> +a cast in atomic_read() and atomic_set(), as well as their 64-bit versions
> +if applicable, since the more complex atomic operations directly or
> +indirectly use assembly that results in volatile behavior.
>
> The first operations to implement for atomic_t's are the
> initializers and plain reads.
>
> #define ATOMIC_INIT(i) { (i) }
> - #define atomic_set(v, i)((v)->counter = (i))
> + #define atomic_set(v, i)(*(volatile int *)&(v)->counter = (i))
>
> The first macro is used in definitions, such as:
>
> @@ -38,7 +45,7 @@
>
> Next, we have:
>
> - #define atomic_read(v) ((v)->counter)
> + #define atomic_read(v) (*(volatile int *)&(v)->counter)
>
> which simply reads the current value of the counter.
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/23] document preferred use of volatile with atomic_t
From: Chris Snook <[EMAIL PROTECTED]>
Document proper use of volatile for atomic_t operations.
Signed-off-by: Chris Snook <[EMAIL PROTECTED]>
--- linux-2.6.23-rc3-orig/Documentation/atomic_ops.txt 2007-07-08
19:32:17.0 -0400
+++ linux-2.6.23-rc3/Documentation/atomic_ops.txt 2007-08-13
03:36:43.0 -0400
@@ -12,13 +12,20 @@
C integer type will fail. Something like the following should
suffice:
- typedef struct { volatile int counter; } atomic_t;
+ typedef struct { int counter; } atomic_t;
+
+ Historically, counter has been declared as a volatile int. This
+is now discouraged in favor of explicitly casting it as volatile where
+volatile behavior is required. Most architectures will only require such
+a cast in atomic_read() and atomic_set(), as well as their 64-bit versions
+if applicable, since the more complex atomic operations directly or
+indirectly use assembly that results in volatile behavior.
The first operations to implement for atomic_t's are the
initializers and plain reads.
#define ATOMIC_INIT(i) { (i) }
- #define atomic_set(v, i)((v)->counter = (i))
+ #define atomic_set(v, i)(*(volatile int *)&(v)->counter = (i))
The first macro is used in definitions, such as:
@@ -38,7 +45,7 @@
Next, we have:
- #define atomic_read(v) ((v)->counter)
+ #define atomic_read(v) (*(volatile int *)&(v)->counter)
which simply reads the current value of the counter.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/23] document preferred use of volatile with atomic_t
From: Chris Snook [EMAIL PROTECTED]
Document proper use of volatile for atomic_t operations.
Signed-off-by: Chris Snook [EMAIL PROTECTED]
--- linux-2.6.23-rc3-orig/Documentation/atomic_ops.txt 2007-07-08
19:32:17.0 -0400
+++ linux-2.6.23-rc3/Documentation/atomic_ops.txt 2007-08-13
03:36:43.0 -0400
@@ -12,13 +12,20 @@
C integer type will fail. Something like the following should
suffice:
- typedef struct { volatile int counter; } atomic_t;
+ typedef struct { int counter; } atomic_t;
+
+ Historically, counter has been declared as a volatile int. This
+is now discouraged in favor of explicitly casting it as volatile where
+volatile behavior is required. Most architectures will only require such
+a cast in atomic_read() and atomic_set(), as well as their 64-bit versions
+if applicable, since the more complex atomic operations directly or
+indirectly use assembly that results in volatile behavior.
The first operations to implement for atomic_t's are the
initializers and plain reads.
#define ATOMIC_INIT(i) { (i) }
- #define atomic_set(v, i)((v)-counter = (i))
+ #define atomic_set(v, i)(*(volatile int *)(v)-counter = (i))
The first macro is used in definitions, such as:
@@ -38,7 +45,7 @@
Next, we have:
- #define atomic_read(v) ((v)-counter)
+ #define atomic_read(v) (*(volatile int *)(v)-counter)
which simply reads the current value of the counter.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/23] document preferred use of volatile with atomic_t
On Mon, Aug 13, 2007 at 07:04:15AM -0400, Chris Snook wrote:
From: Chris Snook [EMAIL PROTECTED]
Document proper use of volatile for atomic_t operations.
Looks good, as did a once-over on the arch-specific files. Good stuff!!!
Acked-by: Paul E. McKenney [EMAIL PROTECTED]
Signed-off-by: Chris Snook [EMAIL PROTECTED]
--- linux-2.6.23-rc3-orig/Documentation/atomic_ops.txt2007-07-08
19:32:17.0 -0400
+++ linux-2.6.23-rc3/Documentation/atomic_ops.txt 2007-08-13
03:36:43.0 -0400
@@ -12,13 +12,20 @@
C integer type will fail. Something like the following should
suffice:
- typedef struct { volatile int counter; } atomic_t;
+ typedef struct { int counter; } atomic_t;
+
+ Historically, counter has been declared as a volatile int. This
+is now discouraged in favor of explicitly casting it as volatile where
+volatile behavior is required. Most architectures will only require such
+a cast in atomic_read() and atomic_set(), as well as their 64-bit versions
+if applicable, since the more complex atomic operations directly or
+indirectly use assembly that results in volatile behavior.
The first operations to implement for atomic_t's are the
initializers and plain reads.
#define ATOMIC_INIT(i) { (i) }
- #define atomic_set(v, i)((v)-counter = (i))
+ #define atomic_set(v, i)(*(volatile int *)(v)-counter = (i))
The first macro is used in definitions, such as:
@@ -38,7 +45,7 @@
Next, we have:
- #define atomic_read(v) ((v)-counter)
+ #define atomic_read(v) (*(volatile int *)(v)-counter)
which simply reads the current value of the counter.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
