Nick Piggin wrote:
> Satyam Sharma wrote:
>> And we have driver / subsystem maintainers such as Stefan
>> coming up and admitting that often a lot of code that's written to use
>> atomic_read() does assume the read will not be elided by the compiler.
>
> So these are broken on i386 and x86-64?
Th
I wrote:
> Nick Piggin wrote:
>> You might find that these places that appear to need barriers are
>> buggy for other reasons anyway. Can you point to some in-tree code
>> we can have a look at?
>
> I could, or could not, if I were through with auditing the code. I
> remembered one case and poste
Nick Piggin wrote:
> Stefan Richter wrote:
>> For architecture port authors, there is Documentation/atomic_ops.txt.
>> Driver authors also can learn something from that document, as it
>> indirectly documents the atomic_t and bitops APIs.
>
> "Semantics and Behavior of Atomic and Bitmask Operation
On Fri, 17 Aug 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
> > [...]
> > The point is about *author expecations*. If people do expect atomic_read()
> > (or a variant thereof) to have volatile semantics, why not give them such
> > a variant?
>
> Because they should be thinking about them in
On Fri, 17 Aug 2007, Paul Mackerras wrote:
> Satyam Sharma writes:
>
> > I wonder if this'll generate smaller and better code than _both_ the
> > other atomic_read_volatile() variants. Would need to build allyesconfig
> > on lots of diff arch's etc to test the theory though.
>
> I'm sure it wo
On Fri, 17 Aug 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
> >
> > On Fri, 17 Aug 2007, Nick Piggin wrote:
>
> > > Also, why would you want to make these insane accessors for atomic_t
> > > types? Just make sure everybody knows the basics of barriers, and they
> > > can apply that knowledg
On Fri, 17 Aug 2007, Andi Kleen wrote:
> On Friday 17 August 2007 05:42, Linus Torvalds wrote:
> > On Fri, 17 Aug 2007, Paul Mackerras wrote:
> > > I'm really surprised it's as much as a few K. I tried it on powerpc
> > > and it only saved 40 bytes (10 instructions) for a G5 config.
> >
> > One
On Fri, 17 Aug 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
> >
> > On Fri, 17 Aug 2007, Nick Piggin wrote:
>
> > > > Sure, now
> > > > that I learned of these properties I can start to audit code and insert
> > > > barriers where I believe they are needed, but this simply means that
> > >
Satyam Sharma writes:
> I wonder if this'll generate smaller and better code than _both_ the
> other atomic_read_volatile() variants. Would need to build allyesconfig
> on lots of diff arch's etc to test the theory though.
I'm sure it would be a tiny effect.
This whole thread is arguing about ef
Satyam Sharma wrote:
On Fri, 17 Aug 2007, Nick Piggin wrote:
Satyam Sharma wrote:
[...]
Granted, the above IS buggy code. But, the stated objective is to avoid
heisenbugs.
^^
Anyway, why are you making up code snippets that are buggy in other
ways in order to support this a
Satyam Sharma wrote:
On Fri, 17 Aug 2007, Nick Piggin wrote:
Also, why would you want to make these insane accessors for atomic_t
types? Just make sure everybody knows the basics of barriers, and they
can apply that knowledge to atomic_t and all other lockless memory
accesses as well.
Code
Satyam Sharma wrote:
On Fri, 17 Aug 2007, Nick Piggin wrote:
Sure, now
that I learned of these properties I can start to audit code and insert
barriers where I believe they are needed, but this simply means that
almost all occurrences of atomic_read will get barriers (unless there
already are
On Fri, 17 Aug 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
> [...]
> > Granted, the above IS buggy code. But, the stated objective is to avoid
> > heisenbugs.
^^
> Anyway, why are you making up code snippets that are buggy in other
> ways in order to support this assertion being
On Friday 17 August 2007 05:42, Linus Torvalds wrote:
> On Fri, 17 Aug 2007, Paul Mackerras wrote:
> > I'm really surprised it's as much as a few K. I tried it on powerpc
> > and it only saved 40 bytes (10 instructions) for a G5 config.
>
> One of the things that "volatile" generally screws up is
On Fri, 17 Aug 2007, Nick Piggin wrote:
> Stefan Richter wrote:
> [...]
> Just use spinlocks if you're not absolutely clear about potential
> races and memory ordering issues -- they're pretty cheap and simple.
I fully agree with this. As Paul Mackerras mentioned elsewhere,
a lot of authors spr
Satyam Sharma wrote:
On Fri, 17 Aug 2007, Herbert Xu wrote:
On Fri, Aug 17, 2007 at 01:43:27PM +1000, Paul Mackerras wrote:
BTW, the sort of missing barriers that triggered this thread
aren't that subtle. It'll result in a simple lock-up if the
loop condition holds upon entry. At which poi
On Fri, 17 Aug 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
>
> > #define atomic_read_volatile(v) \
> > ({ \
> > forget((v)->counter); \
> > ((v)->counter);
On Fri, 17 Aug 2007, Paul Mackerras wrote:
> Herbert Xu writes:
>
> > On Fri, Aug 17, 2007 at 03:09:57PM +1000, Paul Mackerras wrote:
> > > Herbert Xu writes:
> > >
> > > > Can you find an actual atomic_read code snippet there that is
> > > > broken without the volatile modifier?
> > >
> > >
Stefan Richter wrote:
Nick Piggin wrote:
I don't know why people would assume volatile of atomics. AFAIK, most
of the documentation is pretty clear that all the atomic stuff can be
reordered etc. except for those that modify and return a value.
Which documentation is there?
Documentation/a
Nick Piggin wrote:
> I don't know why people would assume volatile of atomics. AFAIK, most
> of the documentation is pretty clear that all the atomic stuff can be
> reordered etc. except for those that modify and return a value.
Which documentation is there?
For driver authors, there is LDD3. It
On Thu, 16 Aug 2007, Paul E. McKenney wrote:
> On Fri, Aug 17, 2007 at 07:59:02AM +0800, Herbert Xu wrote:
> > On Thu, Aug 16, 2007 at 09:34:41AM -0700, Paul E. McKenney wrote:
> > >
> > > The compiler can also reorder non-volatile accesses. For an example
> > > patch that cares about this, ple
Satyam Sharma wrote:
#define atomic_read_volatile(v) \
({ \
forget((v)->counter);\
((v)->counter); \
})
where:
*vomit* :)
On Thu, 16 Aug 2007, Linus Torvalds wrote:
> On Fri, 17 Aug 2007, Paul Mackerras wrote:
> > I'm really surprised it's as much as a few K. I tried it on powerpc
> > and it only saved 40 bytes (10 instructions) for a G5 config.
>
> One of the things that "volatile" generally screws up is a simple
>
On Fri, 17 Aug 2007, Herbert Xu wrote:
> On Fri, Aug 17, 2007 at 01:43:27PM +1000, Paul Mackerras wrote:
> >
> > The cost of doing so seems to me to be well down in the noise - 44
> > bytes of extra kernel text on a ppc64 G5 config, and I don't believe
> > the extra few cycles for the occasional
Hi Linus,
[ and others; I think there's a communication gap in a lot of this
thread, and a little summary would be useful. Hence this posting. ]
On Thu, 16 Aug 2007, Linus Torvalds wrote:
> On Fri, 17 Aug 2007, Paul Mackerras wrote:
> >
> > I'm really surprised it's as much as a few K. I tr
Herbert Xu writes:
> On Fri, Aug 17, 2007 at 03:09:57PM +1000, Paul Mackerras wrote:
> > Herbert Xu writes:
> >
> > > Can you find an actual atomic_read code snippet there that is
> > > broken without the volatile modifier?
> >
> > There are some in arch-specific code, for example line 1073 of
>
On Fri, Aug 17, 2007 at 03:09:57PM +1000, Paul Mackerras wrote:
> Herbert Xu writes:
>
> > Can you find an actual atomic_read code snippet there that is
> > broken without the volatile modifier?
>
> There are some in arch-specific code, for example line 1073 of
> arch/mips/kernel/smtc.c. On mips
On Thu, Aug 16, 2007 at 08:42:23PM -0700, Linus Torvalds wrote:
>
>
> On Fri, 17 Aug 2007, Paul Mackerras wrote:
> >
> > I'm really surprised it's as much as a few K. I tried it on powerpc
> > and it only saved 40 bytes (10 instructions) for a G5 config.
>
> One of the things that "volatile" g
Herbert Xu writes:
> Can you find an actual atomic_read code snippet there that is
> broken without the volatile modifier?
There are some in arch-specific code, for example line 1073 of
arch/mips/kernel/smtc.c. On mips, cpu_relax() is just barrier(), so
the empty loop body is ok provided that at
On Fri, Aug 17, 2007 at 09:28:00AM +0800, Herbert Xu wrote:
> On Thu, Aug 16, 2007 at 06:02:32PM -0700, Paul E. McKenney wrote:
> >
> > Yep. Or you can use atomic_dec_return() instead of using a barrier.
>
> Or you could use smp_mb__{before,after}_atomic_dec.
Yep. That would be an example of a
Herbert Xu writes:
> So the point here is that if you don't mind getting a stale
> value from the CPU cache when doing an atomic_read, then
> surely you won't mind getting a stale value from the compiler
> "cache".
No, that particular argument is bogus, because there is a cache
coherency protocol
Paul Mackerras wrote:
Nick Piggin writes:
Why are people making these undocumented and just plain false
assumptions about atomic_t?
Well, it has only been false since December 2006. Prior to that
atomics *were* volatile on all platforms.
Hmm, although I don't think it has ever been guara
[ Your mailer drops Cc: lists, munges headers,
does all sorts of badness. Please fix that. ]
On Thu, 16 Aug 2007, David Schwartz wrote:
>
> > There is a quite convincing argument that such an access _is_ an
> > access to a volatile object; see GCC PR21568 comment #9. This
> > probably isn't
On Thu, 16 Aug 2007, Segher Boessenkool wrote:
> > Here, I should obviously admit that the semantics of *(volatile int *)&
> > aren't any neater or well-defined in the _language standard_ at all. The
> > standard does say (verbatim) "precisely what constitutes as access to
> > object of volatile
On Thu, 16 Aug 2007, Segher Boessenkool wrote:
> > Note that "volatile"
> > is a type-qualifier, not a type itself, so a cast of the _object_ itself
> > to a qualified-type i.e. (volatile int) would not make the access itself
> > volatile-qualified.
>
> There is no such thing as "volatile-quali
Nick Piggin writes:
> Why are people making these undocumented and just plain false
> assumptions about atomic_t?
Well, it has only been false since December 2006. Prior to that
atomics *were* volatile on all platforms.
> If they're using lockless code (ie.
> which they must be if using atomics
On Fri, Aug 17, 2007 at 01:43:27PM +1000, Paul Mackerras wrote:
>
> The cost of doing so seems to me to be well down in the noise - 44
> bytes of extra kernel text on a ppc64 G5 config, and I don't believe
> the extra few cycles for the occasional extra load would be measurable
> (they should all h
On Fri, 17 Aug 2007, Nick Piggin wrote:
>
> I'm surprised too. Numbers were from the "...use asm() like the other
> atomic operations already do" thread. According to them,
>
> textdata bss dec hex filename
> 3434150 249176 176128 3859454 3ae3fe atomic_normal/vmlinux
> 3436
Linus Torvalds writes:
> In general, I'd *much* rather we used barriers. Anything that "depends" on
> volatile is pretty much set up to be buggy. But I'm certainly also willing
> to have that volatile inside "atomic_read/atomic_set()" if it avoids code
> that would otherwise break - ie if it hi
On Fri, 17 Aug 2007, Paul Mackerras wrote:
>
> I'm really surprised it's as much as a few K. I tried it on powerpc
> and it only saved 40 bytes (10 instructions) for a G5 config.
One of the things that "volatile" generally screws up is a simple
volatile int i;
i++;
which a c
Paul Mackerras wrote:
Nick Piggin writes:
So i386 and x86-64 don't have volatiles there, and it saves them a
few K of kernel text. What you need to justify is why it is a good
I'm really surprised it's as much as a few K. I tried it on powerpc
and it only saved 40 bytes (10 instructions) f
Nick Piggin writes:
> So i386 and x86-64 don't have volatiles there, and it saves them a
> few K of kernel text. What you need to justify is why it is a good
I'm really surprised it's as much as a few K. I tried it on powerpc
and it only saved 40 bytes (10 instructions) for a G5 config.
Paul.
-
Paul E. McKenney wrote:
On Thu, Aug 16, 2007 at 06:42:50PM +0800, Herbert Xu wrote:
In fact, volatile doesn't guarantee that the memory gets
read anyway. You might be reading some stale value out
of the cache. Granted this doesn't happen on x86 but
when you're coding for the kernel you can't
On Fri, 17 Aug 2007, Paul Mackerras wrote:
>
> Volatile doesn't mean it can't be reordered; volatile means the
> accesses can't be eliminated.
It also does limit re-ordering.
Of course, since *normal* accesses aren't necessarily limited wrt
re-ordering, the question then becomes one of "with
Christoph Lameter writes:
> No it does not have any volatile semantics. atomic_dec() can be reordered
> at will by the compiler within the current basic unit if you do not add a
> barrier.
Volatile doesn't mean it can't be reordered; volatile means the
accesses can't be eliminated.
Paul.
-
To
Chris Snook wrote:
Herbert Xu wrote:
On Thu, Aug 16, 2007 at 03:48:54PM -0400, Chris Snook wrote:
Can you find an actual atomic_read code snippet there that is
broken without the volatile modifier?
A whole bunch of atomic_read uses will be broken without the volatile
modifier once we start
Segher Boessenkool wrote:
Part of the motivation here is to fix heisenbugs. If I knew where
they
By the same token we should probably disable optimisations
altogether since that too can create heisenbugs.
Almost everything is a tradeoff; and so is this. I don't
believe most people would f
On Thu, Aug 16, 2007 at 10:04:24PM -0400, Chris Snook wrote:
>
> >Could you please cite the file/function names so we can
> >see whether removing the barrier makes sense?
>
> At a glance, several architectures' implementations of smp_call_function()
> have one or more legitimate atomic_read() bus
Herbert Xu wrote:
On Thu, Aug 16, 2007 at 03:48:54PM -0400, Chris Snook wrote:
Can you find an actual atomic_read code snippet there that is
broken without the volatile modifier?
A whole bunch of atomic_read uses will be broken without the volatile
modifier once we start removing barriers that
On Thu, Aug 16, 2007 at 06:02:32PM -0700, Paul E. McKenney wrote:
>
> Yep. Or you can use atomic_dec_return() instead of using a barrier.
Or you could use smp_mb__{before,after}_atomic_dec.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
H
On Thu, Aug 16, 2007 at 01:20:26PM -0700, Christoph Lameter wrote:
> On Thu, 16 Aug 2007, Chris Snook wrote:
>
> > atomic_dec() already has volatile behavior everywhere, so this is
> > semantically
> > okay, but this code (and any like it) should be calling cpu_relax() each
> > iteration through
On Fri, Aug 17, 2007 at 07:59:02AM +0800, Herbert Xu wrote:
> On Thu, Aug 16, 2007 at 09:34:41AM -0700, Paul E. McKenney wrote:
> >
> > The compiler can also reorder non-volatile accesses. For an example
> > patch that cares about this, please see:
> >
> > http://lkml.org/lkml/2007/8/7/280
>
On Thu, Aug 16, 2007 at 03:48:54PM -0400, Chris Snook wrote:
>
> >Can you find an actual atomic_read code snippet there that is
> >broken without the volatile modifier?
>
> A whole bunch of atomic_read uses will be broken without the volatile
> modifier once we start removing barriers that aren't
On Thu, Aug 16, 2007 at 09:34:41AM -0700, Paul E. McKenney wrote:
>
> The compiler can also reorder non-volatile accesses. For an example
> patch that cares about this, please see:
>
> http://lkml.org/lkml/2007/8/7/280
>
> This patch uses an ORDERED_WRT_IRQ() in rcu_read_lock() and
> rcu_r
> There is a quite convincing argument that such an access _is_ an
> access to a volatile object; see GCC PR21568 comment #9. This
> probably isn't the last word on the matter though...
I find this argument completely convincing and retract the contrary argument
that I've made many times in this
>> 6674: while (atomic_read(&j->DSPWrite) > 0)
>> 6675- atomic_dec(&j->DSPWrite);
>
> If the maintainer of this code doesn't see a compelling reason to add
> cpu_relax() in this loop, then it should be patched.
Shouldn't it be just re-written without the loop:
if ((tmp = atom
Here, I should obviously admit that the semantics of *(volatile int *)&
aren't any neater or well-defined in the _language standard_ at all.
The
standard does say (verbatim) "precisely what constitutes as access to
object of volatile-qualified type is implementation-defined", but GCC
does help u
Note that "volatile"
is a type-qualifier, not a type itself, so a cast of the _object_
itself
to a qualified-type i.e. (volatile int) would not make the access
itself
volatile-qualified.
There is no such thing as "volatile-qualified access" defined
anywhere; there only is the concept of a "vo
I can't speak for this particular case, but there could be similar
code
examples elsewhere, where we do the atomic ops on an atomic_t object
inside a higher-level locking scheme that would take care of the kind
of
problem you're referring to here. It would be useful for such or
similar
code if
On Thu, 16 Aug 2007, Chris Snook wrote:
> atomic_dec() already has volatile behavior everywhere, so this is semantically
> okay, but this code (and any like it) should be calling cpu_relax() each
> iteration through the loop, unless there's a compelling reason not to. I'll
> allow that for some h
On Thu, Aug 16, 2007 at 11:54:54AM -0700, Christoph Lameter wrote:
> On Thu, 16 Aug 2007, Paul Mackerras wrote:
> > So I don't see any good reason to make the atomic API more complex by
> > having "volatile" and "non-volatile" versions of atomic_read. It
> > should just have the "volatile" behavio
Ilpo Järvinen wrote:
On Thu, 16 Aug 2007, Herbert Xu wrote:
We've been through that already. If it's a busy-wait it
should use cpu_relax.
I looked around a bit by using some command lines and ended up wondering
if these are equal to busy-wait case (and should be fixed) or not:
./drivers/
Herbert Xu wrote:
On Thu, Aug 16, 2007 at 10:06:31AM +0200, Stefan Richter wrote:
Do you (or anyone else for that matter) have an example of this?
The only code I somewhat know, the ieee1394 subsystem, was perhaps
authored and is currently maintained with the expectation that each
occurrence of
Ilpo Järvinen wrote:
On Thu, 16 Aug 2007, Herbert Xu wrote:
We've been through that already. If it's a busy-wait it
should use cpu_relax.
I looked around a bit by using some command lines and ended up wondering
if these are equal to busy-wait case (and should be fixed) or not:
./drivers/
I'd go so far as to say that anywhere where you want a non-"volatile"
atomic_read, either your code is buggy, or else an int would work just
as well.
Even, the only way to implement a "non-volatile" atomic_read() is
essentially as a plain int (you can do some tricks so you cannot
assign to the r
The only thing volatile on an asm does is create a side effect
on the asm statement; in effect, it tells the compiler "do not
remove this asm even if you don't need any of its outputs".
It's not disabling optimisation likely to result in bugs,
heisen- or otherwise; _not_ putting the volatile on a
Part of the motivation here is to fix heisenbugs. If I knew where
they
By the same token we should probably disable optimisations
altogether since that too can create heisenbugs.
Almost everything is a tradeoff; and so is this. I don't
believe most people would find disabling all compiler
op
On Thu, 16 Aug 2007, Paul Mackerras wrote:
> The uses of atomic_read where one might want it to allow caching of
> the result seem to me to fall into 3 categories:
>
> 1. Places that are buggy because of a race arising from the way it's
>used.
>
> 2. Places where there is a race but it doesn
On Thu, 16 Aug 2007, Paul Mackerras wrote:
> Herbert Xu writes:
>
> > It doesn't matter. The memory pressure flag is an *advisory*
> > flag. If we get it wrong the worst that'll happen is that we'd
> > waste some time doing work that'll be thrown away.
>
> Ah, so it's the "racy but I don't car
On Thu, 16 Aug 2007, Paul Mackerras wrote:
>
> It seems that there could be a lot of places where atomic_t is used in
> a non-atomic fashion, and that those uses are either buggy, or there
> is some lock held at the time which guarantees that other CPUs aren't
> changing the value. In both cases
On Thu, Aug 16, 2007 at 06:42:50PM +0800, Herbert Xu wrote:
> On Thu, Aug 16, 2007 at 12:31:03PM +0200, Stefan Richter wrote:
> >
> > PS: Just to clarify, I'm not speaking for the volatile modifier. I'm
> > not speaking for any particular implementation of atomic_t and its
> > accessors at all.
Ilpo Järvinen wrote:
> I looked around a bit by using some command lines and ended up wondering
> if these are equal to busy-wait case (and should be fixed) or not:
>
> ./drivers/telephony/ixj.c
> 6674: while (atomic_read(&j->DSPWrite) > 0)
> 6675- atomic_dec(&j->DSPWrite);
>
> ...be
On Thu, 16 Aug 2007, Herbert Xu wrote:
> We've been through that already. If it's a busy-wait it
> should use cpu_relax.
I looked around a bit by using some command lines and ended up wondering
if these are equal to busy-wait case (and should be fixed) or not:
./drivers/telephony/ixj.c
6674:
On Thu, Aug 16, 2007 at 12:31:03PM +0200, Stefan Richter wrote:
>
> PS: Just to clarify, I'm not speaking for the volatile modifier. I'm
> not speaking for any particular implementation of atomic_t and its
> accessors at all. All I am saying is that
> - we use atomically accessed data types b
On Thu, Aug 16, 2007 at 11:54:44AM +0200, Stefan Richter wrote:
>
> One example was discussed here earlier: The for (;;) loop in
> nodemgr_host_thread. There an msleep_interruptible implicitly acted as
> barrier (at the moment because it's in a different translation unit; if
> it were the same,
I wrote:
> Herbert Xu wrote:
>> On Thu, Aug 16, 2007 at 10:06:31AM +0200, Stefan Richter wrote:
[...]
>>> expectation that each
>>> occurrence of atomic_read actually results in a load operation, i.e. is
>>> not optimized away.
[...]
>> Can you find an actual atomic_read code snippet there that is
Herbert Xu wrote:
> On Thu, Aug 16, 2007 at 10:06:31AM +0200, Stefan Richter wrote:
>> >
>> > Do you (or anyone else for that matter) have an example of this?
>>
>> The only code I somewhat know, the ieee1394 subsystem, was perhaps
>> authored and is currently maintained with the expectation that
[ Bill tells me in private communication he gets this already, but I
think it's more complicated than the shoddy explanation I'd made
earlier so would wish to make this clearer in detail one last time,
for the benefit of others listening in or reading the archives. ]
On Thu, 16 Aug 2007, Sa
On Thu, Aug 16, 2007 at 10:06:31AM +0200, Stefan Richter wrote:
> >
> > Do you (or anyone else for that matter) have an example of this?
>
> The only code I somewhat know, the ieee1394 subsystem, was perhaps
> authored and is currently maintained with the expectation that each
> occurrence of ato
Herbert Xu wrote:
> On Thu, Aug 16, 2007 at 04:56:21PM +1000, Paul Mackerras wrote:
>>
>> Note that I said these are the cases _where one might want to allow
>> caching_, so of course adding volatile doesn't help _these_ cases.
>> There are of course other cases where one definitely doesn't want to
On Thu, Aug 16, 2007 at 04:56:21PM +1000, Paul Mackerras wrote:
>
> Note that I said these are the cases _where one might want to allow
> caching_, so of course adding volatile doesn't help _these_ cases.
> There are of course other cases where one definitely doesn't want to
> allow the compiler to
Herbert Xu writes:
> On Thu, Aug 16, 2007 at 02:11:43PM +1000, Paul Mackerras wrote:
> >
> > The uses of atomic_read where one might want it to allow caching of
> > the result seem to me to fall into 3 categories:
> >
> > 1. Places that are buggy because of a race arising from the way it's
> >
Herbert Xu writes:
> It doesn't matter. The memory pressure flag is an *advisory*
> flag. If we get it wrong the worst that'll happen is that we'd
> waste some time doing work that'll be thrown away.
Ah, so it's the "racy but I don't care because it's only an
optimization" case. That's fine.
On Thu, 16 Aug 2007, Satyam Sharma wrote:
> Hi Bill,
>
>
> On Wed, 15 Aug 2007, Bill Fink wrote:
>
> > On Wed, 15 Aug 2007, Satyam Sharma wrote:
> >
> > > (C)
> > > $ cat tp3.c
> > > int a;
> > >
> > > void func(void)
> > > {
> > > *(volatile int *)&a = 10;
> > > *(volatile int *)&a = 2
On Thu, Aug 16, 2007 at 02:11:43PM +1000, Paul Mackerras wrote:
>
> The uses of atomic_read where one might want it to allow caching of
> the result seem to me to fall into 3 categories:
>
> 1. Places that are buggy because of a race arising from the way it's
>used.
>
> 2. Places where there
On Thu, Aug 16, 2007 at 02:34:25PM +1000, Paul Mackerras wrote:
>
> I'm talking about this situation:
>
> CPU 0 comes into __sk_stream_mem_reclaim, reads memory_allocated, but
> then before it can do the store to *memory_pressure, CPUs 1-1023 all
> go through sk_stream_mem_schedule, collectively i
Hi Bill,
On Wed, 15 Aug 2007, Bill Fink wrote:
> On Wed, 15 Aug 2007, Satyam Sharma wrote:
>
> > (C)
> > $ cat tp3.c
> > int a;
> >
> > void func(void)
> > {
> > *(volatile int *)&a = 10;
> > *(volatile int *)&a = 20;
> > }
> > $ gcc -Os -S tp3.c
> > $ cat tp3.s
> > ...
> > movl$10
Herbert Xu writes:
> > You mean it's intended that *sk->sk_prot->memory_pressure can end up
> > as 1 when sk->sk_prot->memory_allocated is small (less than
> > ->sysctl_mem[0]), or as 0 when ->memory_allocated is large (greater
> > than ->sysctl_mem[2])? Because that's the effect of the current c
Satyam Sharma writes:
> Anyway, the problem, of course, is that this conversion to a stronger /
> safer-by-default behaviour doesn't happen with zero cost to performance.
> Converting atomic ops to "volatile" behaviour did add ~2K to kernel text
> for archs such as i386 (possibly to important code
On Thu, Aug 16, 2007 at 01:48:32PM +1000, Paul Mackerras wrote:
> Herbert Xu writes:
>
> > If you're referring to the code in sk_stream_mem_schedule
> > then it's working as intended. The atomicity guarantees
>
> You mean it's intended that *sk->sk_prot->memory_pressure can end up
> as 1 when sk
Herbert Xu writes:
> If you're referring to the code in sk_stream_mem_schedule
> then it's working as intended. The atomicity guarantees
You mean it's intended that *sk->sk_prot->memory_pressure can end up
as 1 when sk->sk_prot->memory_allocated is small (less than
->sysctl_mem[0]), or as 0 when
On Wed, 15 Aug 2007, Satyam Sharma wrote:
> (C)
> $ cat tp3.c
> int a;
>
> void func(void)
> {
> *(volatile int *)&a = 10;
> *(volatile int *)&a = 20;
> }
> $ gcc -Os -S tp3.c
> $ cat tp3.s
> ...
> movl$10, a
> movl$20, a
> ...
I'm curious about one minor tangential point. W
On Thu, Aug 16, 2007 at 01:15:05PM +1000, Paul Mackerras wrote:
>
> But others can also reduce the reservation. Also, the code sets and
> clears *sk->sk_prot->memory_pressure nonatomically with respect to the
> reads of sk->sk_prot->memory_allocated, so in fact the code doesn't
> guarantee any pa
On Thu, Aug 16, 2007 at 01:23:06PM +1000, Paul Mackerras wrote:
>
> In particular, atomic_read seems to lend itself to buggy uses. People
> seem to do things like:
>
> atomic_add(&v, something);
> if (atomic_read(&v) > something_else) ...
If you're referring to the code in sk_stream_
Herbert Xu writes:
> > Are you sure? How do you know some other CPU hasn't changed the value
> > in between?
>
> Yes I'm sure, because we don't care if others have increased
> the reservation.
But others can also reduce the reservation. Also, the code sets and
clears *sk->sk_prot->memory_press
Christoph Lameter writes:
> > But I have to say that I still don't know of a single place
> > where one would actually use the volatile variant.
>
> I suspect that what you say is true after we have looked at all callers.
It seems that there could be a lot of places where atomic_t is used in
a n
Satyam Sharma writes:
> I can't speak for this particular case, but there could be similar code
> examples elsewhere, where we do the atomic ops on an atomic_t object
> inside a higher-level locking scheme that would take care of the kind of
> problem you're referring to here. It would be useful f
On Thu, 16 Aug 2007, Satyam Sharma wrote:
> On Thu, 16 Aug 2007, Paul Mackerras wrote:
> > Herbert Xu writes:
> >
> > > See sk_stream_mem_schedule in net/core/stream.c:
> > >
> > > /* Under limit. */
> > > if (atomic_read(sk->sk_prot->memory_allocated) <
> > > sk->sk_prot->sys
On Thu, Aug 16, 2007 at 10:11:05AM +0800, Herbert Xu wrote:
> On Thu, Aug 16, 2007 at 12:05:56PM +1000, Paul Mackerras wrote:
> > Herbert Xu writes:
> >
> > > See sk_stream_mem_schedule in net/core/stream.c:
> > >
> > > /* Under limit. */
> > > if (atomic_read(sk->sk_prot->memory_
On Wed, Aug 15, 2007 at 06:41:40PM -0700, Christoph Lameter wrote:
> On Wed, 15 Aug 2007, Paul E. McKenney wrote:
>
> > Understood. My point is not that the impact is precisely zero, but
> > rather that the impact on optimization is much less hurtful than the
> > problems that could arise otherwi
101 - 200 of 293 matches
Mail list logo
