question about code from the linux kernel development ( se ) book

question about code from the linux kernel development ( se ) book

[Yitzchak Eidus]

first i am very sorry if it isnt the place to ask questions like this
but i didnt know where else to ask ( i tryed irc channels and i was
send from there to this list )
anyway:
does this following code look buggy? :
DECLARE_WAITQUEUE ( wait , current );
add_wait_queue ( q , &wait );
while ( !condition ) {
set_current_stat ( TASK_INTERRUPTABLE ); i
if ( signal_pending ( current ) )
/* handle signal */
schedule ( ); }
set_current_state ( TASK_RUNNING );
remove_wait_queue ( q , &wait );
first:doesnt in the way from checking the !condition to
set_current_state  the condition can be changed no?

second:why not putting the schedule ( ); right after the
set_current_state ( ) , what the point in checking the if (
signal_pending ( ) first, if the proccess doesnt started to sleep yet?
third: in the cleaning in the way from putting the set_current_state (
TASK_RUNNING ) into remove_wait_queue , cant the queue wait list ( q )
wake up again the wait procsess?
( thnks for the help , please if it can be done answer quickly i am
tanker in the idf and need to come back to the army soon , ( no
internet there... ) )
thnks!

Re: question about code from the linux kernel development ( se ) book

[Steven Rostedt]

Oh God, please indent code examples, and if your email client strips white
space, change your email client.

On Fri, 21 Oct 2005, Yitzchak Eidus wrote:

> first i am very sorry if it isnt the place to ask questions like this
> but i didnt know where else to ask ( i tryed irc channels and i was
> send from there to this list )
> anyway:
> does this following code look buggy? :

[ Indention added ]

> DECLARE_WAITQUEUE ( wait , current );
> add_wait_queue ( q , &wait );
> while ( !condition ) {
>         set_current_stat ( TASK_INTERRUPTABLE ); i
>         if ( signal_pending ( current ) )
>                 /* handle signal */

I assume that the signal_pending is the if result and not the schedule.
Since there was no indentation I couldn't tell.

>         schedule ( );
/* Moved brace down added */
 }
> set_current_state ( TASK_RUNNING );
> remove_wait_queue ( q , &wait );

Before I go to your questions, I'll first answer that this _is_ buggy
code.  If the condition is checked and you are woken up between the
while (!condition) and the set_current_state, then you will end up
sleeping forever or until someone sends you a signal.

> first:doesnt in the way from checking the !condition to
> set_current_state  the condition can be changed no?

Yes, and then put it again after schedule.

>
> second:why not putting the schedule ( ); right after the
> set_current_state ( ) , what the point in checking the if (
> signal_pending ( ) first, if the proccess doesnt started to sleep yet?

Yes, I would put the signal_pending check after the schedule (as most of
the kernel does this).

> third: in the cleaning in the way from putting the set_current_state (
> TASK_RUNNING ) into remove_wait_queue , cant the queue wait list ( q )
> wake up again the wait procsess?

Yeah, so?  There's no harm in that, except for an extra cpu cycles that
are done to wake it up.  That, I wouldn't change.

> ( thnks for the help , please if it can be done answer quickly i am
> tanker in the idf and need to come back to the army soon , ( no
> internet there... ) )

BTW, this is not an IRC, we use normal capitalization and normal spelling
(when we know how to spell a word ;-).  So the next time you send to the
list, send it as if you were writing a serious letter, or you may just be
ignored. (as you might have been if I didn't respond).

-- Steve

Re: question about code from the linux kernel development ( se ) book

[Jesper Juhl]

On 10/21/05, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Oh God, please indent code examples, and if your email client strips white
> space, change your email client.
>
> On Fri, 21 Oct 2005, Yitzchak Eidus wrote:
>
> > first i am very sorry if it isnt the place to ask questions like this
> > but i didnt know where else to ask ( i tryed irc channels and i was
> > send from there to this list )
> > anyway:
> > does this following code look buggy? :
>
> [ Indention added ]
>
Thanks.

> > DECLARE_WAITQUEUE ( wait , current );
> > add_wait_queue ( q , &wait );
> > while ( !condition ) {
> >         set_current_stat ( TASK_INTERRUPTABLE ); i

It is probably just a typo, but that "i" at the end of the line would be a bug.


--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: question about code from the linux kernel development ( se ) book

[Yitzchak Eidus]

First Steve I want to thanks you for answering me
your answered all the questions that i had
I can understand your complains , and i will try to make it better
next time ( i almost never used mailing lists... , so sorry about the
white space striping , and the spelling... )

Jesper - yes it was a typo

Nick - thanks :)

On 10/21/05, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Oh God, please indent code examples, and if your email client strips white
> space, change your email client.
>
> On Fri, 21 Oct 2005, Yitzchak Eidus wrote:
>
> > first i am very sorry if it isnt the place to ask questions like this
> > but i didnt know where else to ask ( i tryed irc channels and i was
> > send from there to this list )
> > anyway:
> > does this following code look buggy? :
>
> [ Indention added ]
>
> > DECLARE_WAITQUEUE ( wait , current );
> > add_wait_queue ( q , &wait );
> > while ( !condition ) {
> >         set_current_stat ( TASK_INTERRUPTABLE ); i
> >         if ( signal_pending ( current ) )
> >                 /* handle signal */
>
> I assume that the signal_pending is the if result and not the schedule.
> Since there was no indentation I couldn't tell.
>
> >         schedule ( );
> /* Moved brace down added */
>  }
> > set_current_state ( TASK_RUNNING );
> > remove_wait_queue ( q , &wait );
>
> Before I go to your questions, I'll first answer that this _is_ buggy
> code.  If the condition is checked and you are woken up between the
> while (!condition) and the set_current_state, then you will end up
> sleeping forever or until someone sends you a signal.
>
> > first:doesnt in the way from checking the !condition to
> > set_current_state  the condition can be changed no?
>
> Yes, and then put it again after schedule.
>
> >
> > second:why not putting the schedule ( ); right after the
> > set_current_state ( ) , what the point in checking the if (
> > signal_pending ( ) first, if the proccess doesnt started to sleep yet?
>
> Yes, I would put the signal_pending check after the schedule (as most of
> the kernel does this).
>
> > third: in the cleaning in the way from putting the set_current_state (
> > TASK_RUNNING ) into remove_wait_queue , cant the queue wait list ( q )
> > wake up again the wait procsess?
>
> Yeah, so?  There's no harm in that, except for an extra cpu cycles that
> are done to wake it up.  That, I wouldn't change.
>
> > ( thnks for the help , please if it can be done answer quickly i am
> > tanker in the idf and need to come back to the army soon , ( no
> > internet there... ) )
>
> BTW, this is not an IRC, we use normal capitalization and normal spelling
> (when we know how to spell a word ;-).  So the next time you send to the
> list, send it as if you were writing a serious letter, or you may just be
> ignored. (as you might have been if I didn't respond).
>
> -- Steve
>

Re: question about code from the linux kernel development ( se ) book

[Steven Rostedt]

On Fri, 21 Oct 2005, Yitzchak Eidus wrote:

> First Steve I want to thanks you for answering me
> your answered all the questions that i had
> I can understand your complains , and i will try to make it better
> next time ( i almost never used mailing lists... , so sorry about the
> white space striping , and the spelling... )
>
> Jesper - yes it was a typo
>
> Nick - thanks :)
>

Hi Yitzchak,

No problem.  Here's a link to read to know how to reply to this list.
This isn't quite the same as most mailing lists because of the volume of
mail one receives here. (I get around 300 a day). So here's part of the
FAQ in sending to this list.  One thing to notice is that you shouldn't
"top-post".  That is, to reply above what you are replying to and leave
the entire message intact. ;-)

http://www.tux.org/lkml/#s3-9

-- Steve

Re: question about code from the linux kernel development ( se ) book

[Lee Revell]

On Fri, 2005-10-21 at 09:03 -0400, Steven Rostedt wrote:
> Oh God, please indent code examples, and if your email client strips white
> space, change your email client.

Or better yet PLEASE REPORT THIS AS A BUG IN YOUR EMAIL CLIENT.
Otherwise we're going to be plagued with this crap forever.

Lee

Re: question about code from the linux kernel development ( se ) book

[Nick Warne]

Steven Rostedt wrote:

>> ( thnks for the help , please if it can be done answer quickly i am
>> tanker in the idf and need to come back to the army soon , ( no
>> internet there... ) )
> 
> BTW, this is not an IRC, we use normal capitalization and normal spelling
> (when we know how to spell a word ;-).  So the next time you send to the
> list, send it as if you were writing a serious letter, or you may just be
> ignored. (as you might have been if I didn't respond).

Well, I am a real newbie to Linux coding, but I think the guy tried to do 
right (irc sent him here) - maybe I am not used to the terseness of Kernel 
hackers in their replies sometimes.


Yitzchak: I have the book - by Robert Love.  I see the same code (page 53), 
but, alas I haven't a clue...

My advice is drop a mail to Robert about it - he _is_ the author, and I am 
sure he will answer your query.

Nick
-- 
http://sourceforge.net/projects/quake2plus

"Person who say it cannot be done should not interrupt person doing it."
-Chinese Proverb

Re: question about code from the linux kernel development ( se ) book

[Steven Rostedt]

On Fri, 21 Oct 2005, Nick Warne wrote:

> Steven Rostedt wrote:
>
> >> ( thnks for the help , please if it can be done answer quickly i am
> >> tanker in the idf and need to come back to the army soon , ( no
> >> internet there... ) )
> >
> > BTW, this is not an IRC, we use normal capitalization and normal spelling
> > (when we know how to spell a word ;-).  So the next time you send to the
> > list, send it as if you were writing a serious letter, or you may just be
> > ignored. (as you might have been if I didn't respond).
>
> Well, I am a real newbie to Linux coding, but I think the guy tried to do
> right (irc sent him here) - maybe I am not used to the terseness of Kernel
> hackers in their replies sometimes.

I didn't say he didn't try to do right.  I didn't mean to come off terse,
I was just informing Yitzchak of the etiquette of this list. I even tried
to soften it with the "how to spell a word" comment.

So Yitzchak, if I offended you, I'm sorry and I didn't mean to do it.

Nick, FYI, Don't parse the CC list. That may be acceptible on other
mailing lists, but the volume of mail on this one, we request that the CC
list remains unless someone asks to be taken off. I only saw this mail
because Yitzchak referenced you in his reply. ;-)

-- Steve