Re: Why does test_bit() take a volatile addr?
Rob Landley writes: > On 09/15/2013 11:08:35 PM, Rusty Russell wrote: >> Predates git, does anyone remember the rationale? > > Depends which git: http://landley.net/kdocs/fullhist/ :) Not useful. See Geert's more helpful response. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On 09/15/2013 11:08:35 PM, Rusty Russell wrote: Predates git, does anyone remember the rationale? Depends which git: http://landley.net/kdocs/fullhist/ :) Rob-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On 09/15/2013 11:08:35 PM, Rusty Russell wrote: Predates git, does anyone remember the rationale? Depends which git: http://landley.net/kdocs/fullhist/ :) Rob-- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
Rob Landley r...@landley.net writes: On 09/15/2013 11:08:35 PM, Rusty Russell wrote: Predates git, does anyone remember the rationale? Depends which git: http://landley.net/kdocs/fullhist/ :) Not useful. See Geert's more helpful response. Cheers, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 12:08 AM, Rusty Russell wrote: > Predates git, does anyone remember the rationale? > > ie: > int test_bit(int nr, const volatile unsigned long *addr) Both of Stephen Rothwell's guesses are correct. One reason is that we used to use "volatile" a lot more than we do now, and "const volatile *" is the most permissive pointer that allows any use without warnings. We've largely stopped using "volatile" in favor of explicit barriers and locks (ie "cpu_relax()" and "barrier()") and explicit volatility in code (ACCESS_ONCE() and "rcu_access_pointer()" etc). The other reasons is for fear of having some old code that does effectively while (condition) /* nothing */ using "test_bit()", and forcing a reload. They used to happen. They were rare even before, and I'd hope they are nonexistent now, but they were real. We could try to see what happens if we remove "volatile" from the bitops these days. But the scary part is all the random drivers potentially doing that second thing. So it's not exactly easily testable. It would need to be worth it to bother. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, 2013-09-16 at 11:44 +0300, Michael S. Tsirkin wrote: > On Mon, Sep 16, 2013 at 10:40:00AM +0200, Oliver Neukum wrote: > > On Mon, 2013-09-16 at 13:38 +0930, Rusty Russell wrote: > > > Predates git, does anyone remember the rationale? > > > > > > ie: > > > int test_bit(int nr, const volatile unsigned long *addr) > > > > > > I noticed because gcc failed to elimiate some code in a patch I was > > > playing with. > > > > > > I'm nervous about subtle bugs involved in ripping it out, even if noone > > > knows why. Should I add __test_bit()? > > > > It seems to me that if you do > > > > b = *ptr & 0xf; > > c = b << 2; > > if (test_bit(1, ptr)) > > > > the compiler could optimize away the memory access on ptr without > > the volatile. We'd have to add a lot of mb(). > > > > Regards > > Oliver > > What is this code supposed to do? > Any specific examples? > Often you see while (test_bit(...) && condition) ... ; If the compiler can show that you don't change the memory you do the test_bit on, it can change this to: if (test_bit(...)) while (condition) ...; That must not happen. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 06:02:31PM +1000, Stephen Rothwell wrote:
> Hi Michael,
>
> On Mon, 16 Sep 2013 10:26:03 +0300 "Michael S. Tsirkin"
> wrote:
> >
> > On Mon, Sep 16, 2013 at 04:53:44PM +1000, Stephen Rothwell wrote:
> > >
> > > On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell
> > > wrote:
> > > >
> > > > Predates git, does anyone remember the rationale?
> > > >
> > > > ie:
> > > > int test_bit(int nr, const volatile unsigned long *addr)
> > >
> > > Because we sometimes pass volatile pointers to it and gcc will complain
> > > if you pass a volatile to a non volatile (I think).
> >
> > Where are these? I did git grep -W test_bit and looked for volatile,
> > couldn't find anything.
>
> OK, so it was a bit of a guess. Have you really checked the type of
> every address passed to every call of test_bit()?
Yea, I have this magic tool called gcc :)
Change
-static __always_inline int constant_test_bit(long nr, const volatile unsigned
long *addr)
+static __always_inline int constant_test_bit(long nr, const unsigned long
*addr)
and watch for new warnings.
I didn't see any.
> Second guess: we wanted to make the test_bit access volatile (as opposed
> to the datatypes of the objects being tested) so that things like
>
> while (testbit(bit, addr)) {
> do_very_little();
> }
>
> don't get over optimised (since we are operating in a very threaded
> environment that the compiler not might expect).
>
> --
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 10:40:00AM +0200, Oliver Neukum wrote: > On Mon, 2013-09-16 at 13:38 +0930, Rusty Russell wrote: > > Predates git, does anyone remember the rationale? > > > > ie: > > int test_bit(int nr, const volatile unsigned long *addr) > > > > I noticed because gcc failed to elimiate some code in a patch I was > > playing with. > > > > I'm nervous about subtle bugs involved in ripping it out, even if noone > > knows why. Should I add __test_bit()? > > It seems to me that if you do > > b = *ptr & 0xf; > c = b << 2; > if (test_bit(1, ptr)) > > the compiler could optimize away the memory access on ptr without > the volatile. We'd have to add a lot of mb(). > > Regards > Oliver What is this code supposed to do? Any specific examples? -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, 2013-09-16 at 13:38 +0930, Rusty Russell wrote: > Predates git, does anyone remember the rationale? > > ie: > int test_bit(int nr, const volatile unsigned long *addr) > > I noticed because gcc failed to elimiate some code in a patch I was > playing with. > > I'm nervous about subtle bugs involved in ripping it out, even if noone > knows why. Should I add __test_bit()? It seems to me that if you do b = *ptr & 0xf; c = b << 2; if (test_bit(1, ptr)) the compiler could optimize away the memory access on ptr without the volatile. We'd have to add a lot of mb(). Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 01:38:35PM +0930, Rusty Russell wrote:
> Predates git, does anyone remember the rationale?
>
> ie:
> int test_bit(int nr, const volatile unsigned long *addr)
>
> I noticed because gcc failed to elimiate some code in a patch I was
> playing with.
>
> I'm nervous about subtle bugs involved in ripping it out, even if noone
> knows why. Should I add __test_bit()?
>
> Thanks,
> Rusty.
So looking at this some more, e.g. on x86 I see:
static inline int variable_test_bit(long nr, volatile const unsigned
long *addr)
{
int oldbit;
asm volatile("bt %2,%1\n\t"
"sbb %0,%0"
: "=r" (oldbit)
: "m" (*(unsigned long *)addr), "Ir" (nr));
return oldbit;
}
and I have a vague memory that (at least for some old versions) gcc
would assume (*(unsigned long *)addr) only refers to addr[0].
OTOH constant_test_bit is
static __always_inline int constant_test_bit(long nr, const volatile unsigned
long *addr)
{
return ((1UL << (nr & (BITS_PER_LONG-1))) &
(addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
}
So there's a chance that we can drop volatile here.
I'll look at it some more.
--
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 6:08 AM, Rusty Russell wrote:
> Predates git, does anyone remember the rationale?
>
> ie:
> int test_bit(int nr, const volatile unsigned long *addr)
That's why we have full-history-linux ;-)
Unfortunately it doesn't show the rationale, as this change also predates
bitkeeper.
Since 2.1.30, volatile is used unconditionally:
commit 84d59b7dda1092a22663f4e2da77a6ce581b539e
Author: linus1
Date: Mon Mar 10 12:00:00 1997 -0600
Import 2.1.30
#ifdef __SMP__
#define LOCK_PREFIX "lock ; "
-#define SMPVOL volatile
#else
#define LOCK_PREFIX ""
-#define SMPVOL
#endif
/*
* This routine doesn't need to be atomic.
*/
-extern __inline__ int test_bit(int nr, const SMPVOL void * addr)
+extern __inline__ int __constant_test_bit(int nr, const volatile void * addr)
{
- return ((1UL << (nr & 31)) & (((const unsigned int *) addr)[nr >> 5])) !
+ return ((1UL << (nr & 31)) & (((const volatile unsigned int *) addr)[nr
}
+extern __inline__ int __test_bit(int nr, volatile void * addr)
+{
+ int oldbit;
+
+ __asm__ __volatile__(
+ "btl %2,%1\n\tsbbl %0,%0"
+ :"=r" (oldbit)
+ :"m" (ADDR),"ir" (nr));
+ return oldbit;
+}
+
+#define test_bit(nr,addr) \
+(__builtin_constant_p(nr) ? \
+ __constant_test_bit((nr),(addr)) : \
+ __test_bit((nr),(addr)))
+
/*
Between 1.3.75 and 2.1.30, volatile was used on SMP only:
commit 08c5b9d698e6ca2233aec0f7d7f0fdd6eb3ad235
Author: linus1
Date: Thu Mar 7 12:00:00 1996 -0600
Import 1.3.75
#ifdef __SMP__
#define LOCK_PREFIX "lock ; "
+#define SMPVOL volatile
#else
#define LOCK_PREFIX ""
+#define SMPVOL
#endif
/*
* This routine doesn't need to be atomic.
*/
-extern __inline__ int test_bit(int nr, const void * addr)
+extern __inline__ int test_bit(int nr, const SMPVOL void * addr)
{
return 1UL & (((const unsigned int *) addr)[nr >> 5] >> (nr & 31));
}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
Hi Michael,
On Mon, 16 Sep 2013 10:26:03 +0300 "Michael S. Tsirkin" wrote:
>
> On Mon, Sep 16, 2013 at 04:53:44PM +1000, Stephen Rothwell wrote:
> >
> > On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell
> > wrote:
> > >
> > > Predates git, does anyone remember the rationale?
> > >
> > > ie:
> > > int test_bit(int nr, const volatile unsigned long *addr)
> >
> > Because we sometimes pass volatile pointers to it and gcc will complain
> > if you pass a volatile to a non volatile (I think).
>
> Where are these? I did git grep -W test_bit and looked for volatile,
> couldn't find anything.
OK, so it was a bit of a guess. Have you really checked the type of
every address passed to every call of test_bit()?
Second guess: we wanted to make the test_bit access volatile (as opposed
to the datatypes of the objects being tested) so that things like
while (testbit(bit, addr)) {
do_very_little();
}
don't get over optimised (since we are operating in a very threaded
environment that the compiler not might expect).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
pgpxq6KdZ4s37.pgp
Description: PGP signature
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 04:53:44PM +1000, Stephen Rothwell wrote: > Hi Rusty, > > On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell > wrote: > > > > Predates git, does anyone remember the rationale? > > > > ie: > > int test_bit(int nr, const volatile unsigned long *addr) > > Because we sometimes pass volatile pointers to it and gcc will complain > if you pass a volatile to a non volatile (I think). Where are these? I did git grep -W test_bit and looked for volatile, couldn't find anything. > -- > Cheers, > Stephen Rothwells...@canb.auug.org.au -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
Hi Rusty, On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell wrote: > > Predates git, does anyone remember the rationale? > > ie: > int test_bit(int nr, const volatile unsigned long *addr) Because we sometimes pass volatile pointers to it and gcc will complain if you pass a volatile to a non volatile (I think). -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpqU5XUhp_0a.pgp Description: PGP signature
Why does test_bit() take a volatile addr?
Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) I noticed because gcc failed to elimiate some code in a patch I was playing with. I'm nervous about subtle bugs involved in ripping it out, even if noone knows why. Should I add __test_bit()? Thanks, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Why does test_bit() take a volatile addr?
Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) I noticed because gcc failed to elimiate some code in a patch I was playing with. I'm nervous about subtle bugs involved in ripping it out, even if noone knows why. Should I add __test_bit()? Thanks, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
Hi Rusty, On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell ru...@rustcorp.com.au wrote: Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) Because we sometimes pass volatile pointers to it and gcc will complain if you pass a volatile to a non volatile (I think). -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpqU5XUhp_0a.pgp Description: PGP signature
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 04:53:44PM +1000, Stephen Rothwell wrote: Hi Rusty, On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell ru...@rustcorp.com.au wrote: Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) Because we sometimes pass volatile pointers to it and gcc will complain if you pass a volatile to a non volatile (I think). Where are these? I did git grep -W test_bit and looked for volatile, couldn't find anything. -- Cheers, Stephen Rothwells...@canb.auug.org.au -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
Hi Michael,
On Mon, 16 Sep 2013 10:26:03 +0300 Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Sep 16, 2013 at 04:53:44PM +1000, Stephen Rothwell wrote:
On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell ru...@rustcorp.com.au
wrote:
Predates git, does anyone remember the rationale?
ie:
int test_bit(int nr, const volatile unsigned long *addr)
Because we sometimes pass volatile pointers to it and gcc will complain
if you pass a volatile to a non volatile (I think).
Where are these? I did git grep -W test_bit and looked for volatile,
couldn't find anything.
OK, so it was a bit of a guess. Have you really checked the type of
every address passed to every call of test_bit()?
Second guess: we wanted to make the test_bit access volatile (as opposed
to the datatypes of the objects being tested) so that things like
while (testbit(bit, addr)) {
do_very_little();
}
don't get over optimised (since we are operating in a very threaded
environment that the compiler not might expect).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
pgpxq6KdZ4s37.pgp
Description: PGP signature
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 6:08 AM, Rusty Russell ru...@rustcorp.com.au wrote:
Predates git, does anyone remember the rationale?
ie:
int test_bit(int nr, const volatile unsigned long *addr)
That's why we have full-history-linux ;-)
Unfortunately it doesn't show the rationale, as this change also predates
bitkeeper.
Since 2.1.30, volatile is used unconditionally:
commit 84d59b7dda1092a22663f4e2da77a6ce581b539e
Author: linus1 torva...@linuxfoundation.org
Date: Mon Mar 10 12:00:00 1997 -0600
Import 2.1.30
#ifdef __SMP__
#define LOCK_PREFIX lock ;
-#define SMPVOL volatile
#else
#define LOCK_PREFIX
-#define SMPVOL
#endif
/*
* This routine doesn't need to be atomic.
*/
-extern __inline__ int test_bit(int nr, const SMPVOL void * addr)
+extern __inline__ int __constant_test_bit(int nr, const volatile void * addr)
{
- return ((1UL (nr 31)) (((const unsigned int *) addr)[nr 5])) !
+ return ((1UL (nr 31)) (((const volatile unsigned int *) addr)[nr
}
+extern __inline__ int __test_bit(int nr, volatile void * addr)
+{
+ int oldbit;
+
+ __asm__ __volatile__(
+ btl %2,%1\n\tsbbl %0,%0
+ :=r (oldbit)
+ :m (ADDR),ir (nr));
+ return oldbit;
+}
+
+#define test_bit(nr,addr) \
+(__builtin_constant_p(nr) ? \
+ __constant_test_bit((nr),(addr)) : \
+ __test_bit((nr),(addr)))
+
/*
Between 1.3.75 and 2.1.30, volatile was used on SMP only:
commit 08c5b9d698e6ca2233aec0f7d7f0fdd6eb3ad235
Author: linus1 torva...@linuxfoundation.org
Date: Thu Mar 7 12:00:00 1996 -0600
Import 1.3.75
#ifdef __SMP__
#define LOCK_PREFIX lock ;
+#define SMPVOL volatile
#else
#define LOCK_PREFIX
+#define SMPVOL
#endif
/*
* This routine doesn't need to be atomic.
*/
-extern __inline__ int test_bit(int nr, const void * addr)
+extern __inline__ int test_bit(int nr, const SMPVOL void * addr)
{
return 1UL (((const unsigned int *) addr)[nr 5] (nr 31));
}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 01:38:35PM +0930, Rusty Russell wrote:
Predates git, does anyone remember the rationale?
ie:
int test_bit(int nr, const volatile unsigned long *addr)
I noticed because gcc failed to elimiate some code in a patch I was
playing with.
I'm nervous about subtle bugs involved in ripping it out, even if noone
knows why. Should I add __test_bit()?
Thanks,
Rusty.
So looking at this some more, e.g. on x86 I see:
static inline int variable_test_bit(long nr, volatile const unsigned
long *addr)
{
int oldbit;
asm volatile(bt %2,%1\n\t
sbb %0,%0
: =r (oldbit)
: m (*(unsigned long *)addr), Ir (nr));
return oldbit;
}
and I have a vague memory that (at least for some old versions) gcc
would assume (*(unsigned long *)addr) only refers to addr[0].
OTOH constant_test_bit is
static __always_inline int constant_test_bit(long nr, const volatile unsigned
long *addr)
{
return ((1UL (nr (BITS_PER_LONG-1)))
(addr[nr _BITOPS_LONG_SHIFT])) != 0;
}
So there's a chance that we can drop volatile here.
I'll look at it some more.
--
MST
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, 2013-09-16 at 13:38 +0930, Rusty Russell wrote: Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) I noticed because gcc failed to elimiate some code in a patch I was playing with. I'm nervous about subtle bugs involved in ripping it out, even if noone knows why. Should I add __test_bit()? It seems to me that if you do b = *ptr 0xf; c = b 2; if (test_bit(1, ptr)) the compiler could optimize away the memory access on ptr without the volatile. We'd have to add a lot of mb(). Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 10:40:00AM +0200, Oliver Neukum wrote: On Mon, 2013-09-16 at 13:38 +0930, Rusty Russell wrote: Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) I noticed because gcc failed to elimiate some code in a patch I was playing with. I'm nervous about subtle bugs involved in ripping it out, even if noone knows why. Should I add __test_bit()? It seems to me that if you do b = *ptr 0xf; c = b 2; if (test_bit(1, ptr)) the compiler could optimize away the memory access on ptr without the volatile. We'd have to add a lot of mb(). Regards Oliver What is this code supposed to do? Any specific examples? -- MST -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 06:02:31PM +1000, Stephen Rothwell wrote:
Hi Michael,
On Mon, 16 Sep 2013 10:26:03 +0300 Michael S. Tsirkin m...@redhat.com
wrote:
On Mon, Sep 16, 2013 at 04:53:44PM +1000, Stephen Rothwell wrote:
On Mon, 16 Sep 2013 13:38:35 +0930 Rusty Russell ru...@rustcorp.com.au
wrote:
Predates git, does anyone remember the rationale?
ie:
int test_bit(int nr, const volatile unsigned long *addr)
Because we sometimes pass volatile pointers to it and gcc will complain
if you pass a volatile to a non volatile (I think).
Where are these? I did git grep -W test_bit and looked for volatile,
couldn't find anything.
OK, so it was a bit of a guess. Have you really checked the type of
every address passed to every call of test_bit()?
Yea, I have this magic tool called gcc :)
Change
-static __always_inline int constant_test_bit(long nr, const volatile unsigned
long *addr)
+static __always_inline int constant_test_bit(long nr, const unsigned long
*addr)
and watch for new warnings.
I didn't see any.
Second guess: we wanted to make the test_bit access volatile (as opposed
to the datatypes of the objects being tested) so that things like
while (testbit(bit, addr)) {
do_very_little();
}
don't get over optimised (since we are operating in a very threaded
environment that the compiler not might expect).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, 2013-09-16 at 11:44 +0300, Michael S. Tsirkin wrote: On Mon, Sep 16, 2013 at 10:40:00AM +0200, Oliver Neukum wrote: On Mon, 2013-09-16 at 13:38 +0930, Rusty Russell wrote: Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) I noticed because gcc failed to elimiate some code in a patch I was playing with. I'm nervous about subtle bugs involved in ripping it out, even if noone knows why. Should I add __test_bit()? It seems to me that if you do b = *ptr 0xf; c = b 2; if (test_bit(1, ptr)) the compiler could optimize away the memory access on ptr without the volatile. We'd have to add a lot of mb(). Regards Oliver What is this code supposed to do? Any specific examples? Often you see while (test_bit(...) condition) ... ; If the compiler can show that you don't change the memory you do the test_bit on, it can change this to: if (test_bit(...)) while (condition) ...; That must not happen. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why does test_bit() take a volatile addr?
On Mon, Sep 16, 2013 at 12:08 AM, Rusty Russell ru...@rustcorp.com.au wrote: Predates git, does anyone remember the rationale? ie: int test_bit(int nr, const volatile unsigned long *addr) Both of Stephen Rothwell's guesses are correct. One reason is that we used to use volatile a lot more than we do now, and const volatile * is the most permissive pointer that allows any use without warnings. We've largely stopped using volatile in favor of explicit barriers and locks (ie cpu_relax() and barrier()) and explicit volatility in code (ACCESS_ONCE() and rcu_access_pointer() etc). The other reasons is for fear of having some old code that does effectively while (condition) /* nothing */ using test_bit(), and forcing a reload. They used to happen. They were rare even before, and I'd hope they are nonexistent now, but they were real. We could try to see what happens if we remove volatile from the bitops these days. But the scary part is all the random drivers potentially doing that second thing. So it's not exactly easily testable. It would need to be worth it to bother. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
