* volatile variable @ 2003-08-01 10:57 Dinesh Gandhewar 2003-08-01 11:38 ` Richard B. Johnson 0 siblings, 1 reply; 10+ messages in thread From: Dinesh Gandhewar @ 2003-08-01 10:57 UTC (permalink / raw) To: mlist-linux-kernel Hello, If a system call is having following code. add current process into wait quque ; while (1) { set task state INTERRUPTIBLE ; if (a > 0) break ; schedule() ; } set task state RUNNING ; remove current from wait queue ; If an interrupt service is having following code set a = 512 ; 'a' is a global variable shared in ISR and system call Do I need to define a as 'volatile int a ;' ? Why? Thanks & Regards, Dinesh ___________________________________________________ Download the hottest & happening ringtones here! OR SMS: Top tone to 7333 Click here now: http://sms.rediff.com/cgi-bin/ringtone/ringhome.pl ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-01 10:57 volatile variable Dinesh Gandhewar @ 2003-08-01 11:38 ` Richard B. Johnson 2003-08-11 13:33 ` David Woodhouse 0 siblings, 1 reply; 10+ messages in thread From: Richard B. Johnson @ 2003-08-01 11:38 UTC (permalink / raw) To: Dinesh Gandhewar; +Cc: mlist-linux-kernel On Fri, 1 Aug 2003, Dinesh Gandhewar wrote: > Hello, > > If a system call is having following code. > > add current process into wait quque ; > while (1) > { set task state INTERRUPTIBLE ; > if (a > 0) > break ; > schedule() ; > } > set task state RUNNING ; > remove current from wait queue ; > > > If an interrupt service is having following code > > set a = 512 ; > > 'a' is a global variable shared in ISR and system call > > Do I need to define a as 'volatile int a ;' ? Why? > > Thanks & Regards, > Dinesh > First, there are already procedures available to do just what you seem to want to do, interruptible_sleep_on() and interruptible_sleep_on_timeout(). These take care of the ugly details that can trip up compilers. In any event in your loop, variable 'a', has already been read by the code the compiler generates. There is nothing else in the loop that touches that variable. Therefore the compiler is free to (correctly) assume that whatever it was when it was first read is what it will continue to be. The compiler will therefore optimise it to be a single read and compare. So, the loop will continue forever if 'a' started as zero because the compiler correctly knows that it cannot possibly change in the only execution path that it knows about. To tell the compiler that there are other possible execution paths in which the variable could be modified, you need to declare the variable as "volatile". The variable's storage hasn't changed. The size of the variable hasn't changed. All you have done is told the compiler to read that variable every time because it could have changed. Cheers, Dick Johnson Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips). Note 96.31% of all statistics are fiction. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-01 11:38 ` Richard B. Johnson @ 2003-08-11 13:33 ` David Woodhouse 2003-08-11 14:06 ` Richard B. Johnson 0 siblings, 1 reply; 10+ messages in thread From: David Woodhouse @ 2003-08-11 13:33 UTC (permalink / raw) To: root; +Cc: Dinesh Gandhewar, mlist-linux-kernel On Fri, 2003-08-01 at 12:38, Richard B. Johnson wrote: > First, there are already procedures available to do just > what you seem to want to do, interruptible_sleep_on() and > interruptible_sleep_on_timeout(). These take care of the > ugly details that can trip up compilers. Just in case there are people reading this who don't realise that Richard is trolling -- do not ever use sleep_on() and friends. They _will_ introduce bugs, and hence they _will_ be removed from the kernel some time in the (hopefully not-so-distant) future. > In any event in your loop, variable 'a', has already been > read by the code the compiler generates. There is nothing > else in the loop that touches that variable. Therefore > the compiler is free to (correctly) assume that whatever > it was when it was first read is what it will continue to > be. The compiler will therefore optimise it to be a single > read and compare. So, the loop will continue forever if > 'a' started as zero because the compiler correctly knows > that it cannot possibly change in the only execution > path that it knows about. If 'a' is a local variable that's true. If 'a' is a global as the original poster explicitly declared, then the compiler must assume that function calls (such as the one to schedule()) may modify it, and hence may not optimise away the second and subsequent reads. Therefore, the 'volatile' is not required. Richard, stop taunting the newbies :) -- dwmw2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-11 13:33 ` David Woodhouse @ 2003-08-11 14:06 ` Richard B. Johnson 2003-08-11 14:37 ` Jakub Jelinek ` (4 more replies) 0 siblings, 5 replies; 10+ messages in thread From: Richard B. Johnson @ 2003-08-11 14:06 UTC (permalink / raw) To: David Woodhouse; +Cc: Dinesh Gandhewar, mlist-linux-kernel On Mon, 11 Aug 2003, David Woodhouse wrote: > On Fri, 2003-08-01 at 12:38, Richard B. Johnson wrote: > > First, there are already procedures available to do just > > what you seem to want to do, interruptible_sleep_on() and > > interruptible_sleep_on_timeout(). These take care of the > > ugly details that can trip up compilers. > > Just in case there are people reading this who don't realise that > Richard is trolling -- do not ever use sleep_on() and friends. They > _will_ introduce bugs, and hence they _will_ be removed from the kernel > some time in the (hopefully not-so-distant) future. > The linux-2.4.20 contains 516 references to "sleep_on" in the `drivers` tree. This is hardly a function or macro that will be removed. If there are bugs, they will be fixed, not removed. Any driver that makes its own 'sleep until' function is fundamentally broken. A driver should not 'know' about 'schedule()' or any other such thing. This violates a fundamental rule about keeping opaque operations and/or functions opaque. If course, older drivers do 'know' about schedule(), but that doesn't make them correct. If you intend to replace these 'sleep until' operations, then that's wonderful. However, until you do, it would not be wise to ask anybody to roll their own. And, if you are actually making a replacement, it should be a function, not a macro. This will save a lot of space. Anything that is going to wait is not going to be hurt by the call/return overhead. > > In any event in your loop, variable 'a', has already been > > read by the code the compiler generates. There is nothing > > else in the loop that touches that variable. Therefore > > the compiler is free to (correctly) assume that whatever > > it was when it was first read is what it will continue to > > be. The compiler will therefore optimise it to be a single > > read and compare. So, the loop will continue forever if > > 'a' started as zero because the compiler correctly knows > > that it cannot possibly change in the only execution > > path that it knows about. > > If 'a' is a local variable that's true. If 'a' is a global as the > original poster explicitly declared, then the compiler must assume that > function calls (such as the one to schedule()) may modify it, and hence > may not optimise away the second and subsequent reads. Therefore, the > 'volatile' is not required. > Again, this is incorrect. If you look at the declaration of schedule(), you will note "asmlinkage void schedule(void);". Now look up "asmlinkage" #define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0))) The regparm(0) atttibute tells gcc that schedule() will get any/all of its parameters in registers. Since schedule() receives no parameters, that means that, as far as gcc is concerned, it cannot modify anything. That said, this may be a bug or it may have been added to work around some gcc bug. But, nevertheless, as the declaration stands, schedule() will never modify anything because somebody told gcc it won't. > Richard, stop taunting the newbies :) > > -- Ditto: Cheers, Dick Johnson Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips). Note 96.31% of all statistics are fiction. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-11 14:06 ` Richard B. Johnson @ 2003-08-11 14:37 ` Jakub Jelinek 2003-08-11 14:38 ` David Woodhouse ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Jakub Jelinek @ 2003-08-11 14:37 UTC (permalink / raw) To: Richard B. Johnson; +Cc: David Woodhouse, Dinesh Gandhewar, mlist-linux-kernel On Mon, Aug 11, 2003 at 10:06:36AM -0400, Richard B. Johnson wrote: > > If 'a' is a local variable that's true. If 'a' is a global as the > > original poster explicitly declared, then the compiler must assume that > > function calls (such as the one to schedule()) may modify it, and hence > > may not optimise away the second and subsequent reads. Therefore, the > > 'volatile' is not required. > > > > Again, this is incorrect. If you look at the declaration of schedule(), > you will note "asmlinkage void schedule(void);". Now look up > "asmlinkage" > #define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0))) > > The regparm(0) atttibute tells gcc that schedule() will get any/all > of its parameters in registers. Since schedule() receives no parameters, > that means that, as far as gcc is concerned, it cannot modify > anything. That said, this may be a bug or it may have been added > to work around some gcc bug. But, nevertheless, as the declaration > stands, schedule() will never modify anything because somebody told > gcc it won't. You're wrong. regparm(0) attribute tells GCC to pass all arguments on the stack. Also function argument passing (the only thing this attribute controls) is completely orthogonal to whether a function may modify global variables or not. __attribute__((const)) functions are not allowed to write nor read global memory, __attribute__((pure)) functions are not allowed to write global memory and all other functions are allowed to both read and write global memory. As schedule is neither const nor pure, the compiler must assume a global variable can be changed by schedule(). Jakub ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-11 14:06 ` Richard B. Johnson 2003-08-11 14:37 ` Jakub Jelinek @ 2003-08-11 14:38 ` David Woodhouse 2003-08-11 14:40 ` David Howells ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: David Woodhouse @ 2003-08-11 14:38 UTC (permalink / raw) To: root; +Cc: Dinesh Gandhewar, mlist-linux-kernel I was about to compose a reply contradicting you, but I can't be bothered. Dinesh -- the answer remains: do not use sleep_on() (you could perhaps use wait_event() instead though), and do not gratuitously make your variable volatile. Richard, you're amusing to read, and have hence escaped my killfiles because I sometimes find it amusingly tricky to find your mistake -- sometimes I have to read your messages two or three times before I spot your errors, but they're always there somewhere. It must take a large amount of effort and skill to present arguments which are so close to the truth as to appear authoritative, yet introduce errors which are often so subtle. I cannot imagine that you achieve this so consistently by accident alone, and respect greatly your ability to do this. I appreciate the amusement you provide for those who are familiar with your behaviour -- and interjected on this occasion only because you seemed to be misleading a newbie who wasn't likely to be aware of your history, without even much subtlety to your errors. It may be vaguely amusing for those who are watching from the sidelines -- but in this instance you seem to be being a little unfair :) -- dwmw2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-11 14:06 ` Richard B. Johnson 2003-08-11 14:37 ` Jakub Jelinek 2003-08-11 14:38 ` David Woodhouse @ 2003-08-11 14:40 ` David Howells 2003-08-11 14:49 ` Mike Galbraith 2003-08-11 17:07 ` Robert Love 4 siblings, 0 replies; 10+ messages in thread From: David Howells @ 2003-08-11 14:40 UTC (permalink / raw) To: Richard B. Johnson; +Cc: David Woodhouse, Dinesh Gandhewar, mlist-linux-kernel > > Just in case there are people reading this who don't realise that > > Richard is trolling -- do not ever use sleep_on() and friends. They > > _will_ introduce bugs, and hence they _will_ be removed from the kernel > > some time in the (hopefully not-so-distant) future. > > That's an excellent idea:-) It'd also be nice to sort out all the interruptible vs non-interruptible problems previously discussed. > The linux-2.4.20 contains 516 references to "sleep_on" in the > `drivers` tree. This is hardly a function or macro that will > be removed. That doesn't mean it won't be either - maybe in 2.7. > Any driver that makes its own 'sleep until' function is fundamentally > broken. Yes... they should call schedule() in the correct way to avoid races. > If course, older drivers do 'know' about schedule(), but that doesn't make > them correct. That doesn't make them incorrect, either. > > Again, this is incorrect. If you look at the declaration of schedule(), > you will note "asmlinkage void schedule(void);". Now look up > "asmlinkage" > #define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0))) That's just because schedule() needs to be called from assembly (entry.S). This merely nails the ABI in place for those functions that need to be called from assembly code, so that if someone decides they want to tell the C compiler to use more or less registers for argument passing, then they don't have to fix up all the .S files too. > The regparm(0) atttibute tells gcc that schedule() will get any/all > of its parameters in registers. No it doesn't. It says schedule() will get zero arguments in registers (on the i386 arch anyway). It does, however, mean that EAX, EDX and ECX will all be clobbered - probably. > Since schedule() receives no parameters, that means that, as far as gcc is > concerned, it cannot modify anything. No it doesn't. It just means schedule() takes no parameters. David ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-11 14:06 ` Richard B. Johnson ` (2 preceding siblings ...) 2003-08-11 14:40 ` David Howells @ 2003-08-11 14:49 ` Mike Galbraith 2003-08-11 17:07 ` Robert Love 4 siblings, 0 replies; 10+ messages in thread From: Mike Galbraith @ 2003-08-11 14:49 UTC (permalink / raw) To: root; +Cc: David Woodhouse, Dinesh Gandhewar, mlist-linux-kernel At 10:06 AM 8/11/2003 -0400, Richard B. Johnson wrote: >On Mon, 11 Aug 2003, David Woodhouse wrote: > > > On Fri, 2003-08-01 at 12:38, Richard B. Johnson wrote: > > > First, there are already procedures available to do just > > > what you seem to want to do, interruptible_sleep_on() and > > > interruptible_sleep_on_timeout(). These take care of the > > > ugly details that can trip up compilers. > > > > Just in case there are people reading this who don't realise that > > Richard is trolling -- do not ever use sleep_on() and friends. They > > _will_ introduce bugs, and hence they _will_ be removed from the kernel > > some time in the (hopefully not-so-distant) future. > > > >The linux-2.4.20 contains 516 references to "sleep_on" in the >`drivers` tree. This is hardly a function or macro that will >be removed. If there are bugs, they will be fixed, not removed. They've been declared dead since (grep 'DO NOT use them' patch*) 2.5.48. See include/linux/wait.h for details. -Mike ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable 2003-08-11 14:06 ` Richard B. Johnson ` (3 preceding siblings ...) 2003-08-11 14:49 ` Mike Galbraith @ 2003-08-11 17:07 ` Robert Love 4 siblings, 0 replies; 10+ messages in thread From: Robert Love @ 2003-08-11 17:07 UTC (permalink / raw) To: root; +Cc: David Woodhouse, Dinesh Gandhewar, mlist-linux-kernel On Mon, 2003-08-11 at 07:06, Richard B. Johnson wrote: > The regparm(0) atttibute tells gcc that schedule() will get any/all > of its parameters in registers. Since schedule() receives no parameters, > that means that, as far as gcc is concerned, it cannot modify > anything. That said, this may be a bug or it may have been added > to work around some gcc bug. But, nevertheless, as the declaration > stands, schedule() will never modify anything because somebody told > gcc it won't. No, regparm(0) means "zero parameters in registers", so everything is to be found on the stack. Also, the notion of functions as compiler barriers has nothing to do with arguments. It has to do with global variables and pointers from who-knows-where. All functions are compile barriers, regardless of regparm() magic or the number of parameters, with the exception of functions with the gcc 'pure' keyword. Also, sleep_on() is deprecated and bad. Thanks for trolling today. Robert Love ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: volatile variable @ 2003-08-02 14:52 Harm Verhagen 0 siblings, 0 replies; 10+ messages in thread From: Harm Verhagen @ 2003-08-02 14:52 UTC (permalink / raw) To: lkml >On Fri, 1 Aug 2003, Dinesh Gandhewar wrote: > >> Hello, >> >> If a system call is having following code. >> >> add current process into wait quque ; >> while (1) >> { set task state INTERRUPTIBLE ; >> if (a > 0) >> break ; >> schedule() ; >> } >> set task state RUNNING ; >> remove current from wait queue ; >> >> 'a' is a global variable shared in ISR and system call > Dick Johnson wrote: >In any event in your loop, variable 'a', has already been >read by the code the compiler generates. There is nothing >else in the loop that touches that variable. Therefore >the compiler is free to (correctly) assume that whatever >it was when it was first read is what it will continue to >be. The compiler will therefore optimise it to be a single >read and compare. So, the loop will continue forever if >'a' started as zero because the compiler correctly knows >that it cannot possibly change in the only execution >path that it knows about. This is incorrect. If variable 'a' is a _global_ variable the compiler cannot (and will not) assume it is not changed in the loop. (The function call to schedule() might well change the global, from the compiler point of view) It will be reread every loop, even without beeing volatile. When you have local variables that are/contain pointers to some data, you need to mark those data fields volatie to make sure they get reread. regards, Harm Verhagen -- Harm Verhagen <h.verhagen@chello.nl> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2003-08-11 17:08 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-08-01 10:57 volatile variable Dinesh Gandhewar 2003-08-01 11:38 ` Richard B. Johnson 2003-08-11 13:33 ` David Woodhouse 2003-08-11 14:06 ` Richard B. Johnson 2003-08-11 14:37 ` Jakub Jelinek 2003-08-11 14:38 ` David Woodhouse 2003-08-11 14:40 ` David Howells 2003-08-11 14:49 ` Mike Galbraith 2003-08-11 17:07 ` Robert Love 2003-08-02 14:52 Harm Verhagen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).