Messages in this thread | | | Date | Tue, 24 Jul 2007 11:53:50 +0530 (IST) | From | Satyam Sharma <> | Subject | Re: [PATCH 4/8] i386: bitops: Kill volatile-casting of memory addresses |
| |
On Tue, 24 Jul 2007, Nick Piggin wrote:
> Linus Torvalds wrote: > > > > On Mon, 23 Jul 2007, Satyam Sharma wrote: > > > > > > > [4/8] i386: bitops: Kill volatile-casting of memory addresses > > > > > > This is wrong. > > > > The "const volatile" is so that you can pass an arbitrary pointer. The only > > kind of abritraty pointer is "const volatile". > > > > In other words, the "volatile" has nothing at all to do with whether the > > memory is volatile or not (the same way "const" has nothing to do with it: > > it's purely a C type *safety* issue, exactly the same way "const" is a type > > safety issue. > > > > A "const" on a pointer doesn't mean that the thing it points to cannot > > change. When you pass a source pointer to "strlen()", it doesn't have to be > > constant. But "strlen()" takes a "const" pointer, because it work son > > constant pointers *too*. > > > > Same deal here. > > > > Admittedly this may be mostly historic, but regardless - the "volatiles" are > > right. > > > > Using volatile on *data* is generally considered incorrect and bad taste, > > but using it in situations like this potentially makes sense. > > > > Of course, if we remove all "volatiles" in data in the kernel (with the > > possible exception of "jiffies"), we can then remove them from function > > declarations too, but it should be done in that order. > > Well, regardless, it still forces the function to treat the pointer > target as volatile, won't it? It definitely prevents valid optimisations > that would be useful for me in mm/page_alloc.c where page flags are > being set up or torn down or checked with non-atomic bitops.
Yes, and yes. But I think what he meant there is that we'd need to audit the kernel for all users of set_bit and friends and see if callers actually pass in any _data_ that _is_ volatile. So we have to kill them there first, and then in the function declarations here. I think I'll put that on my long-term todo list, but see below.
> Anyway by type safety, do you mean it will stop the compiler from > warning if a pointer to a volatile is passed to the bitop?
The compiler would start warning for all those cases (passing in a pointer to volatile data, when the bitops have lost the volatile casting from their function declarations), actually. Something like "passing argument discards qualifiers from pointer type" ... but considering I didn't see *any* of those warnings after these patches, I'm confused as to what exactly Linus meant here ... and what exactly do we need to do "kill the volatiles".
> If so, then > why don't we just kill all the volatiles out of here and fix any > warnings that comeup? I doubt there would be many, and of those, some > might show up real synchronisation problems.
Yes, but see above.
> OK, not the i386 functions as much because they are all in asm anwyay, > but in general (btw. why does i386 or any architecture define their own > non-atomic bitops? If the version in asm-generic/bitops/non-atomic.h > is not good enough then surely it is a bug in gcc or that file?)
Satyam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |