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
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.
>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 <[email protected]>
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
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.
> > 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
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
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
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
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