From: Linus Torvalds <torvalds@osdl.org>
To: James Bottomley <James.Bottomley@SteelEye.com>
Cc: Andrew Morton <akpm@osdl.org>, Paul Jackson <pj@sgi.com>,
PARISC list <parisc-linux@lists.parisc-linux.org>,
Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Fix the cpumask rewrite
Date: Sat, 26 Jun 2004 12:11:50 -0700 (PDT) [thread overview]
Message-ID: <Pine.LNX.4.58.0406261203370.16079@ppc970.osdl.org> (raw)
In-Reply-To: <1088276343.1750.105.camel@mulgrave>
On Sat, 26 Jun 2004, James Bottomley wrote:
>
> 1) Volatility is a property of the code path, not the data, which I
> agree with.
Right.
> 2) the bit operators need to operate on volatile data (i.e. need a
> volatile in their prototypes) without gcc issuing a qualifier dropped
> warning.
>
> This I disagree with because we have no volatile data currently in the
> kernel that necessitates this behavour.
Why do you disagree, since
- right now PA-RISC is BUGGY because of the lack of volatile. gcc _does_
optimize and combine accesses since it's an inline function _without_
any volatile specifier on the pointers.
- the volatile _documents_ the fact that the bitops have volatile
behaviour.
> 3) The parisc bit operator implementations as inline functions need to
> have volatile data in their function templates because otherwise gcc
> will not implement them correctly and may optimise them away when it
> shouldn't.
>
> I disagree with this too...although I'm on shakier ground, and I'd
> prefer gcc experts quantify why this happens, but if, on parisc, I look
> at the assembly output of your example
>
> while (test_and_set_bit(xxx, field))
> while (test_bit(xx, field)) /* Nothing */;
It seems the pa-risc optimizer for gcc is somehow broken. I just checked
on x86:
#define test_bit(x,y) \
(!!((1ul << x) & *(y)))
int test(unsigned long *a)
{
while (test_bit(0, a));
}
definitely does not re-load the value. I checked with an inline function
too, same result:
testb $1, (%eax)
.p2align 2,,3
.L2:
jne .L2
ie gcc _does_ optimize this away without the volatile. With the volatile
it turns into
.p2align 2,,3
.L2:
movl (%edx), %eax
testb $1, %al
jne .L2
so there's a clear difference here.
Now, why gcc on pa-risc misses that optimization, I have no clue.
> So my contention is that even without the volatile pointers in our
> implementation, we still correctly treat this code path as having
> volatile (i.e. we test the bits each time around the loop). All the
> addition of the volatile to the cpumask patch does is cause us to emit
> spurious warnings.
That's apparently just you being lucky due to having a broken gcc.
Linus
next prev parent reply other threads:[~2004-06-26 19:13 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-06-26 16:08 [PATCH] Fix the cpumask rewrite James Bottomley
2004-06-26 16:32 ` Linus Torvalds
2004-06-26 16:44 ` Linus Torvalds
2004-06-26 16:46 ` James Bottomley
2004-06-26 16:54 ` Linus Torvalds
2004-06-26 17:18 ` James Bottomley
2004-06-26 18:01 ` Linus Torvalds
2004-06-26 18:28 ` Vojtech Pavlik
2004-06-26 18:54 ` Linus Torvalds
2004-06-26 19:02 ` James Bottomley
2004-06-26 19:13 ` Linus Torvalds
2004-06-26 19:13 ` Vojtech Pavlik
2004-06-26 18:59 ` James Bottomley
2004-06-26 19:11 ` Linus Torvalds [this message]
2004-06-26 19:33 ` James Bottomley
2004-06-28 23:16 ` Jonathan Lundell
2004-07-01 13:11 ` Pavel Machek
2004-07-01 14:07 ` [parisc-linux] " Alan Cox
2004-07-01 16:15 ` Linus Torvalds
2004-07-01 16:52 ` Jeff Garzik
2004-07-01 17:42 ` Linus Torvalds
2004-06-26 22:18 ` Chris Wedgwood
2004-06-26 22:48 ` Linus Torvalds
2004-06-26 22:54 ` [parisc-linux] " Alan Cox
2004-06-27 0:05 ` Chris Wedgwood
2004-06-27 12:00 ` Matthias Urlichs
2004-06-27 22:41 ` Chris Wedgwood
2004-06-28 1:24 ` Matthias Urlichs
2004-06-28 5:42 ` Chris Wedgwood
2004-06-28 6:55 ` Matthias Urlichs
2004-06-28 7:02 ` Chris Wedgwood
2004-06-28 7:19 ` Matthias Urlichs
2004-06-27 14:37 ` Alan Cox
2004-07-01 13:33 ` Pavel Machek
2004-07-01 17:43 ` Chris Wedgwood
2004-06-26 23:37 ` jiffies_64 Chris Wedgwood
2004-06-27 1:55 ` more (insane) jiffies ranting Chris Wedgwood
2004-06-27 17:39 ` Linus Torvalds
2004-06-27 17:39 ` [parisc-linux] " Linus Torvalds
2004-06-27 17:39 ` Linus Torvalds
2004-06-26 16:32 ` [PATCH] Fix the cpumask rewrite Linus Torvalds
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=Pine.LNX.4.58.0406261203370.16079@ppc970.osdl.org \
--to=torvalds@osdl.org \
--cc=James.Bottomley@SteelEye.com \
--cc=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=parisc-linux@lists.parisc-linux.org \
--cc=pj@sgi.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.