Re: Sleeping thread not receive signal until it wakes up

Re: Sleeping thread not receive signal until it wakes up

[Robert Hancock]

Luong Ngo wrote:
> Hi Thomas and Dick,
> I appreciate all the responses. They are very good information to me.
> Actually, it wasn't me working on the driver but it's been there long
> time. I thought I just need to add the signal and signal handling
> part, not expecting it would lead me to the driver space.
> Here is what I have in the driver. Maybe racing condition could happen
> in scenario that the ioctl realease the lock but befor going to sleep,
> the ISR is invoked and call waking up on the queue, hence the ioctl
> will not be waken up since the wak up cal already executed. But I
> believe in our system, this could be tolerant since the hardware would
> keep raising interrupt if the abnormal condition still exists (Due to
> the ioctl being blocked so user app nevers get a chance to service the
> device). But is this the reason why my signal handler not get executed
> at all? Theoretically, according to the Richard Stevens book, I think
> the process should be waken up and received the signal even if it gets
> blocked in the IOCTL call, am i right?

..

> static int ats89_ioctl(struct inode *inode, struct file *file, u_int
> cmd, u_long arg)
> {
> 
>          switch(cmd){
>           case GET_IRQ_CMD: {
>            u32  regMask32;
> 
>           spin_lock_irq(dev->lock);
>           while ((dev->irqMask & dev->irqEvent) == 0) {
>                 // Sleep until board interrupt happens
>                 spin_unlock_irq(dev->lock);
>                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));
>                 if (uncond_wakeup) {
>                     /* don't go back to loop */
>                     break;
>                 }
>                 spin_lock_irq(dev->lock);
>             }

Kernel code does not get pre-empted by signals. If the code needs to be 
interruptible by signals this has to be handled explicitly. 
interruptible_sleep on will stop waiting if your task gets a signal, but 
your code doesn't check the signal_pending flag to know whether it 
should exit the loop. If signal_pending(current) is set after the sleep 
you should likely be returning -ERESTARTSYS to allow the task to handle 
the signal. Then after the signal handler from the task returns, the 
ioctl will get called again.

Also, as was pointed out, you should not use the sleep_on family of 
functions, use the wait_event functions intead. sleep_on is racy, if the 
interrupt happened just before you do the sleep, you'll sit there 
waiting for something that already occurred.

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

On 3/8/07, Robert Hancock <hancockr@shaw.ca> wrote:
> Luong Ngo wrote:
> > Hi Thomas and Dick,
> > I appreciate all the responses. They are very good information to me.
> > Actually, it wasn't me working on the driver but it's been there long
> > time. I thought I just need to add the signal and signal handling
> > part, not expecting it would lead me to the driver space.
> > Here is what I have in the driver. Maybe racing condition could happen
> > in scenario that the ioctl realease the lock but befor going to sleep,
> > the ISR is invoked and call waking up on the queue, hence the ioctl
> > will not be waken up since the wak up cal already executed. But I
> > believe in our system, this could be tolerant since the hardware would
> > keep raising interrupt if the abnormal condition still exists (Due to
> > the ioctl being blocked so user app nevers get a chance to service the
> > device). But is this the reason why my signal handler not get executed
> > at all? Theoretically, according to the Richard Stevens book, I think
> > the process should be waken up and received the signal even if it gets
> > blocked in the IOCTL call, am i right?
>
> ..
>
> > static int ats89_ioctl(struct inode *inode, struct file *file, u_int
> > cmd, u_long arg)
> > {
> >
> >          switch(cmd){
> >           case GET_IRQ_CMD: {
> >            u32  regMask32;
> >
> >           spin_lock_irq(dev->lock);
> >           while ((dev->irqMask & dev->irqEvent) == 0) {
> >                 // Sleep until board interrupt happens
> >                 spin_unlock_irq(dev->lock);
> >                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));
> >                 if (uncond_wakeup) {
> >                     /* don't go back to loop */
> >                     break;
> >                 }
> >                 spin_lock_irq(dev->lock);
> >             }
>
> Kernel code does not get pre-empted by signals. If the code needs to be
> interruptible by signals this has to be handled explicitly.
> interruptible_sleep on will stop waiting if your task gets a signal, but
> your code doesn't check the signal_pending flag to know whether it
> should exit the loop. If signal_pending(current) is set after the sleep
> you should likely be returning -ERESTARTSYS to allow the task to handle
> the signal. Then after the signal handler from the task returns, the
> ioctl will get called again.
>
> Also, as was pointed out, you should not use the sleep_on family of
> functions, use the wait_event functions intead. sleep_on is racy, if the
> interrupt happened just before you do the sleep, you'll sit there
> waiting for something that already occurred.
>
> --
> Robert Hancock      Saskatoon, SK, Canada
> To email, remove "nospam" from hancockr@nospamshaw.ca
> Home Page: http://www.roberthancock.com/
>
>
Robert, thanks a lot for your suggestion
But I have added the signal_pending(current) check and signal handler
is not invoked

           spin_lock_irq(dev->lock);
           while ((dev->irqMask & dev->irqEvent) == 0) {
                 // Sleep until board interrupt happens
                 spin_unlock_irq(dev->lock);
                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));

                 if(signal_pending(current) {
                           return -ERESTARTSYS;
                 }

                 if (uncond_wakeup) {
                     /* don't go back to loop */
                     break;
                 }
                 spin_lock_irq(dev->lock);
             }
Still no luck yet.
LNgo

RE: Sleeping thread not receive signal until it wakes up

[Parav K Pandit]

-----Original Message-----
From: linux-kernel-owner@vger.kernel.org
[mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Luong Ngo
Sent: Friday, March 09, 2007 8:54 AM
To: Robert Hancock
Cc: linux-kernel; tglx@linutronix.de
Subject: Re: Sleeping thread not receive signal until it wakes up

On 3/8/07, Robert Hancock <hancockr@shaw.ca> wrote:
> Luong Ngo wrote:
> > Hi Thomas and Dick,
> > I appreciate all the responses. They are very good information to me.
> > Actually, it wasn't me working on the driver but it's been there long
> > time. I thought I just need to add the signal and signal handling
> > part, not expecting it would lead me to the driver space.
> > Here is what I have in the driver. Maybe racing condition could happen
> > in scenario that the ioctl realease the lock but befor going to sleep,
> > the ISR is invoked and call waking up on the queue, hence the ioctl
> > will not be waken up since the wak up cal already executed. But I
> > believe in our system, this could be tolerant since the hardware would
> > keep raising interrupt if the abnormal condition still exists (Due to
> > the ioctl being blocked so user app nevers get a chance to service the
> > device). But is this the reason why my signal handler not get executed
> > at all? Theoretically, according to the Richard Stevens book, I think
> > the process should be waken up and received the signal even if it gets
> > blocked in the IOCTL call, am i right?
>
> ..
>
> > static int ats89_ioctl(struct inode *inode, struct file *file, u_int
> > cmd, u_long arg)
> > {
> >
> >          switch(cmd){
> >           case GET_IRQ_CMD: {
> >            u32  regMask32;
> >
> >           spin_lock_irq(dev->lock);
> >           while ((dev->irqMask & dev->irqEvent) == 0) {
> >                 // Sleep until board interrupt happens
> >                 spin_unlock_irq(dev->lock);
> >                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));
> >                 if (uncond_wakeup) {
> >                     /* don't go back to loop */
> >                     break;
> >                 }
> >                 spin_lock_irq(dev->lock);
> >             }
>
> Kernel code does not get pre-empted by signals. If the code needs to be
> interruptible by signals this has to be handled explicitly.
> interruptible_sleep on will stop waiting if your task gets a signal, but
> your code doesn't check the signal_pending flag to know whether it
> should exit the loop. If signal_pending(current) is set after the sleep
> you should likely be returning -ERESTARTSYS to allow the task to handle
> the signal. Then after the signal handler from the task returns, the
> ioctl will get called again.
>
> Also, as was pointed out, you should not use the sleep_on family of
> functions, use the wait_event functions intead. sleep_on is racy, if the
> interrupt happened just before you do the sleep, you'll sit there
> waiting for something that already occurred.
>
> --
> Robert Hancock      Saskatoon, SK, Canada
> To email, remove "nospam" from hancockr@nospamshaw.ca
> Home Page: http://www.roberthancock.com/
>
>
> Robert, thanks a lot for your suggestion
> But I have added the signal_pending(current) check and signal handler
> is not invoked

>           spin_lock_irq(dev->lock);
>           while ((dev->irqMask & dev->irqEvent) == 0) {
>                 // Sleep until board interrupt happens
>                 spin_unlock_irq(dev->lock);
>                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));
>
>                 if(signal_pending(current) {
>                           return -ERESTARTSYS;
>                 }
>
>                 if (uncond_wakeup) {
>                     /* don't go back to loop */
>                     break;
>                 }
>                 spin_lock_irq(dev->lock);
>             }
>Still no luck yet.
>LNgo
>-

I guess you need to call allow_signal(xxx) before you go for sleep.
Parav

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/


DISCLAIMER:
This message (including attachment if any) is confidential and may be privileged. Before opening attachments please check them for viruses and defects. MindTree Consulting Limited (MindTree) will not be responsible for any viruses or defects or any forwarded attachments emanating either from within MindTree or outside. If you have received this message by mistake please notify the sender by return  e-mail and delete this message from your system. Any unauthorized use or dissemination of this message in whole or in part is strictly prohibited.  Please note that e-mails are susceptible to change and MindTree shall not be liable for any improper, untimely or incomplete transmission.

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

On 3/8/07, Parav K Pandit <parav_pandit@mindtree.com> wrote:
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Luong Ngo
> Sent: Friday, March 09, 2007 8:54 AM
> To: Robert Hancock
> Cc: linux-kernel; tglx@linutronix.de
> Subject: Re: Sleeping thread not receive signal until it wakes up
>
> On 3/8/07, Robert Hancock <hancockr@shaw.ca> wrote:
> > Luong Ngo wrote:
> > > Hi Thomas and Dick,
> > > I appreciate all the responses. They are very good information to me.
> > > Actually, it wasn't me working on the driver but it's been there long
> > > time. I thought I just need to add the signal and signal handling
> > > part, not expecting it would lead me to the driver space.
> > > Here is what I have in the driver. Maybe racing condition could happen
> > > in scenario that the ioctl realease the lock but befor going to sleep,
> > > the ISR is invoked and call waking up on the queue, hence the ioctl
> > > will not be waken up since the wak up cal already executed. But I
> > > believe in our system, this could be tolerant since the hardware would
> > > keep raising interrupt if the abnormal condition still exists (Due to
> > > the ioctl being blocked so user app nevers get a chance to service the
> > > device). But is this the reason why my signal handler not get executed
> > > at all? Theoretically, according to the Richard Stevens book, I think
> > > the process should be waken up and received the signal even if it gets
> > > blocked in the IOCTL call, am i right?
> >
> > ..
> >
> > > static int ats89_ioctl(struct inode *inode, struct file *file, u_int
> > > cmd, u_long arg)
> > > {
> > >
> > >          switch(cmd){
> > >           case GET_IRQ_CMD: {
> > >            u32  regMask32;
> > >
> > >           spin_lock_irq(dev->lock);
> > >           while ((dev->irqMask & dev->irqEvent) == 0) {
> > >                 // Sleep until board interrupt happens
> > >                 spin_unlock_irq(dev->lock);
> > >                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));
> > >                 if (uncond_wakeup) {
> > >                     /* don't go back to loop */
> > >                     break;
> > >                 }
> > >                 spin_lock_irq(dev->lock);
> > >             }
> >
> > Kernel code does not get pre-empted by signals. If the code needs to be
> > interruptible by signals this has to be handled explicitly.
> > interruptible_sleep on will stop waiting if your task gets a signal, but
> > your code doesn't check the signal_pending flag to know whether it
> > should exit the loop. If signal_pending(current) is set after the sleep
> > you should likely be returning -ERESTARTSYS to allow the task to handle
> > the signal. Then after the signal handler from the task returns, the
> > ioctl will get called again.
> >
> > Also, as was pointed out, you should not use the sleep_on family of
> > functions, use the wait_event functions intead. sleep_on is racy, if the
> > interrupt happened just before you do the sleep, you'll sit there
> > waiting for something that already occurred.
> >
> > --
> > Robert Hancock      Saskatoon, SK, Canada
> > To email, remove "nospam" from hancockr@nospamshaw.ca
> > Home Page: http://www.roberthancock.com/
> >
> >
> > Robert, thanks a lot for your suggestion
> > But I have added the signal_pending(current) check and signal handler
> > is not invoked
>
> >           spin_lock_irq(dev->lock);
> >           while ((dev->irqMask & dev->irqEvent) == 0) {
> >                 // Sleep until board interrupt happens
> >                 spin_unlock_irq(dev->lock);
> >                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));
> >
> >                 if(signal_pending(current) {
> >                           return -ERESTARTSYS;
> >                 }
> >
> >                 if (uncond_wakeup) {
> >                     /* don't go back to loop */
> >                     break;
> >                 }
> >                 spin_lock_irq(dev->lock);
> >             }
> >Still no luck yet.
> >LNgo
> >-
>
> I guess you need to call allow_signal(xxx) before you go for sleep.
> Parav
>
> 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/
>
>
> DISCLAIMER:
> This message (including attachment if any) is confidential and may be privileged. Before opening attachments please check them for viruses and defects. MindTree Consulting Limited (MindTree) will not be responsible for any viruses or defects or any forwarded attachments emanating either from within MindTree or outside. If you have received this message by mistake please notify the sender by return  e-mail and delete this message from your system. Any unauthorized use or dissemination of this message in whole or in part is strictly prohibited.  Please note that e-mails are susceptible to change and MindTree shall not be liable for any improper, untimely or incomplete transmission.
>
Thanks Parav, adding singal_allow(SIGALRM) wakeup the blocking
interruptible_sleep_on and checking the signal_pending would return
true now. But a quick question is when detecting signal_pending, I
return -ERESTARTSYS, but the user process receive the return value of
ioctl call is -1, shouldn't it be -4 for EINTR?

Thanks all for the responses,
-LNgo

Re: Sleeping thread not receive signal until it wakes up

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]

On Fri, 9 Mar 2007 16:10:29 -0800 Luong Ngo wrote:

> Thanks Parav, adding singal_allow(SIGALRM) wakeup the blocking
> interruptible_sleep_on and checking the signal_pending would return
> true now.

This means that there is also a bug in your userspace program -
somehow when it invokes ioctl(), it has SIGALRM blocked.  Use
sigprocmask() (or pthread_sigmask() if your program is multithreaded)
to ensure that SIGALRM is not blocked when you are expecting it to be
processed.  (Even if your program does not block SIGALRM, it may
inherit a blocked SIGALRM from another program which have started it,
so the safest way is to unblock SIGALRM explicitly.)

Using allow_signal() is needed only if you create a kernel thread and
want that thread to handle signals.

> But a quick question is when detecting signal_pending, I
> return -ERESTARTSYS, but the user process receive the return value of
> ioctl call is -1, shouldn't it be -4 for EINTR?

This is handled by syscall wrappers in libc - when the kernel returns
an error code from a syscall, the syscall wrapper function puts the
code into the per-thread errno variable and returns -1 to its caller.
Your userspace code should check errno for the actual error code when
it receives -1 from ioctl().

And the ERESTARTSYS error is really special - it never actually gets
returned to the userspace program; instead, the syscall return code in
the kernel catches it and either restarts the syscall after the signal
handler completes, or converts the error code to EINTR; one of these
actions is selected by the SA_RESTART flag to sigaction().

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

On 3/10/07, Sergey Vlasov <vsu@altlinux.ru> wrote:
> On Fri, 9 Mar 2007 16:10:29 -0800 Luong Ngo wrote:
>
> > Thanks Parav, adding singal_allow(SIGALRM) wakeup the blocking
> > interruptible_sleep_on and checking the signal_pending would return
> > true now.
>
> This means that there is also a bug in your userspace program -
> somehow when it invokes ioctl(), it has SIGALRM blocked.  Use
> sigprocmask() (or pthread_sigmask() if your program is multithreaded)
> to ensure that SIGALRM is not blocked when you are expecting it to be
> processed.  (Even if your program does not block SIGALRM, it may
> inherit a blocked SIGALRM from another program which have started it,
> so the safest way is to unblock SIGALRM explicitly.)
>
When creating the thread, which would make ioctl call, the parent
thread does not call any sigprocmask so I assume that means none of
the signals is masked. Hence SIGALRM should be received by the thread.
But this did not happen until I call allow_signal explicitly in the
ioctl handler of the driver. I did not call sigprocmask with
SIG_BLOCK.

> Using allow_signal() is needed only if you create a kernel thread and
> want that thread to handle signals.
>
Could the execution of ioctl in the driver be done in a kernel thread
that created to handle the user ioctl request and therefore it needs
allow_signal?


Thanks a lot,
LNgo

Sleeping thread not receive signal until it wakes up

[Luong Ngo]

Hi all,

I am having this problem. I have a process with 2 threads created. One
of the thread will keep calling IOCTL  to get information from the
kernel and will be blocked if there is no new information. If there is
information retured, the thread will be checked to see if any error
happens and trigger an action. Since we have no way to know if the
error is gone (Hardware provides no signal), so what we do is when
trigger an action for the error, we will set an timer using alarm()
and register a SIGALRM handler in the thread by using sigaction. After
setting the alarm, the thread will loop back and call IOCTL, which
could cause it to be put to sleep. The problem is the SIGALRM handler
does not receive the SIGALRM while the thread is being blocked by
IOCTL. And if we generated some event so that the IOCTL is returned
with new information, the SIGALRM handler is invoked right away.
However, as I read the manual, which says a thread/process should be
waken up even when it sleeps if there is a signal delivered to it. Am
I right?
One thing I don't know it mattters or not is that I am not using
sigwait to block the process and wait for signal because the thread
need to go back to the IOCTL call and be slept on that. So I used
sigaction to register the signal handler in hope that this handler wil
be invoked by the kernel when there is an SIGALRM delivered to the
thread.
Could anyone tell me if I did something wrong and what is the correct
way to achieve this task? I tried to avoid creating another thread
which will call sigwait and block until the IOCTL thread send it
explicitly a signal because I want to use timer.


Thank you in advance,
LNgo

Re: Sleeping thread not receive signal until it wakes up

[linux-os (Dick Johnson)]

On Wed, 7 Mar 2007, Luong Ngo wrote:

> Hi all,
>
> I am having this problem. I have a process with 2 threads created. One
> of the thread will keep calling IOCTL  to get information from the
> kernel and will be blocked if there is no new information. If there is
> information retured, the thread will be checked to see if any error
> happens and trigger an action. Since we have no way to know if the
> error is gone (Hardware provides no signal), so what we do is when
> trigger an action for the error, we will set an timer using alarm()
> and register a SIGALRM handler in the thread by using sigaction. After
> setting the alarm, the thread will loop back and call IOCTL, which
> could cause it to be put to sleep. The problem is the SIGALRM handler
> does not receive the SIGALRM while the thread is being blocked by
> IOCTL. And if we generated some event so that the IOCTL is returned
> with new information, the SIGALRM handler is invoked right away.
> However, as I read the manual, which says a thread/process should be
> waken up even when it sleeps if there is a signal delivered to it. Am
> I right?
> One thing I don't know it mattters or not is that I am not using
> sigwait to block the process and wait for signal because the thread
> need to go back to the IOCTL call and be slept on that. So I used
> sigaction to register the signal handler in hope that this handler wil
> be invoked by the kernel when there is an SIGALRM delivered to the
> thread.
> Could anyone tell me if I did something wrong and what is the correct
> way to achieve this task? I tried to avoid creating another thread
> which will call sigwait and block until the IOCTL thread send it
> explicitly a signal because I want to use timer.
>
>
> Thank you in advance,
> LNgo
> -

Later versions of the kernel lock the kernel when an ioctl() is
entered. This means that if you sleep in the ioctl(), nothing
will get scheduled.

You can do the following (possibly unsafe) in your ioctl():

     int locked = kernel_locked();

     ......... code
     ......... code

     if(locked)  // Before sleeping section
        unlock_kernel();
     .......... sleeping code
     if(locked)  // After sleeping section
         lock_kernel();



Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
New book: http://www.AbominableFirebug.com/
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

Hi Dick,
Thanks for your response. In my ioctl in the kernel, I use
interruptible_sleep_on to sleep on a queue and will be wake up by the
the ISR routine when interrupt happens, so  isn't
interruptible_sleep_on supposed to be interruptable, from its name? I
am using kernel 2.6.14.

Thanks again,
LNgo

On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
>
> On Wed, 7 Mar 2007, Luong Ngo wrote:
>
> > Hi all,
> >
> > I am having this problem. I have a process with 2 threads created. One
> > of the thread will keep calling IOCTL  to get information from the
> > kernel and will be blocked if there is no new information. If there is
> > information retured, the thread will be checked to see if any error
> > happens and trigger an action. Since we have no way to know if the
> > error is gone (Hardware provides no signal), so what we do is when
> > trigger an action for the error, we will set an timer using alarm()
> > and register a SIGALRM handler in the thread by using sigaction. After
> > setting the alarm, the thread will loop back and call IOCTL, which
> > could cause it to be put to sleep. The problem is the SIGALRM handler
> > does not receive the SIGALRM while the thread is being blocked by
> > IOCTL. And if we generated some event so that the IOCTL is returned
> > with new information, the SIGALRM handler is invoked right away.
> > However, as I read the manual, which says a thread/process should be
> > waken up even when it sleeps if there is a signal delivered to it. Am
> > I right?
> > One thing I don't know it mattters or not is that I am not using
> > sigwait to block the process and wait for signal because the thread
> > need to go back to the IOCTL call and be slept on that. So I used
> > sigaction to register the signal handler in hope that this handler wil
> > be invoked by the kernel when there is an SIGALRM delivered to the
> > thread.
> > Could anyone tell me if I did something wrong and what is the correct
> > way to achieve this task? I tried to avoid creating another thread
> > which will call sigwait and block until the IOCTL thread send it
> > explicitly a signal because I want to use timer.
> >
> >
> > Thank you in advance,
> > LNgo
> > -
>
> Later versions of the kernel lock the kernel when an ioctl() is
> entered. This means that if you sleep in the ioctl(), nothing
> will get scheduled.
>
> You can do the following (possibly unsafe) in your ioctl():
>
>     int locked = kernel_locked();
>
>     ......... code
>     ......... code
>
>     if(locked)  // Before sleeping section
>        unlock_kernel();
>     .......... sleeping code
>     if(locked)  // After sleeping section
>         lock_kernel();
>
>
>
> Cheers,
> Dick Johnson
> Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
> New book: http://www.AbominableFirebug.com/
> _
> \x1a\x04
>
> ****************************************************************
> The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.
>
> Thank you.
>

Re: Sleeping thread not receive signal until it wakes up

[linux-os (Dick Johnson)]

>
> On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
>>
>> On Wed, 7 Mar 2007, Luong Ngo wrote:
>>
>>> Hi all,
>>>
>>> I am having this problem. I have a process with 2 threads created. One
>>> of the thread will keep calling IOCTL  to get information from the
>>> kernel and will be blocked if there is no new information. If there is
>>> information retured, the thread will be checked to see if any error
>>> happens and trigger an action. Since we have no way to know if the
>>> error is gone (Hardware provides no signal), so what we do is when
>>> trigger an action for the error, we will set an timer using alarm()
>>> and register a SIGALRM handler in the thread by using sigaction. After
>>> setting the alarm, the thread will loop back and call IOCTL, which
>>> could cause it to be put to sleep. The problem is the SIGALRM handler
>>> does not receive the SIGALRM while the thread is being blocked by
>>> IOCTL. And if we generated some event so that the IOCTL is returned
>>> with new information, the SIGALRM handler is invoked right away.
>>> However, as I read the manual, which says a thread/process should be
>>> waken up even when it sleeps if there is a signal delivered to it. Am
>>> I right?
>>> One thing I don't know it mattters or not is that I am not using
>>> sigwait to block the process and wait for signal because the thread
>>> need to go back to the IOCTL call and be slept on that. So I used
>>> sigaction to register the signal handler in hope that this handler wil
>>> be invoked by the kernel when there is an SIGALRM delivered to the
>>> thread.
>>> Could anyone tell me if I did something wrong and what is the correct
>>> way to achieve this task? I tried to avoid creating another thread
>>> which will call sigwait and block until the IOCTL thread send it
>>> explicitly a signal because I want to use timer.
>>>
>>>
>>> Thank you in advance,
>>> LNgo
>>> -
>>
>> Later versions of the kernel lock the kernel when an ioctl() is
>> entered. This means that if you sleep in the ioctl(), nothing
>> will get scheduled.
>>
>> You can do the following (possibly unsafe) in your ioctl():
>>
>>     int locked = kernel_locked();
>>
>>     ......... code
>>     ......... code
>>
>>     if(locked)  // Before sleeping section
>>        unlock_kernel();
>>     .......... sleeping code
>>     if(locked)  // After sleeping section
>>         lock_kernel();
>>
>>
>>
>> Cheers,
>> Dick Johnson
>> Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
>> New book: http://www.AbominableFirebug.com/
>> _
>>
>> Thank you.
>>
On Wed, 7 Mar 2007, Luong Ngo wrote:

> Hi Dick,
> Thanks for your response. In my ioctl in the kernel, I use
> interruptible_sleep_on to sleep on a queue and will be wake up by the
> the ISR routine when interrupt happens, so  isn't
> interruptible_sleep_on supposed to be interruptable, from its name? I
> am using kernel 2.6.14.
>
> Thanks again,
> LNgo

Please don't "top post," you need to put answers at the bottom.

Interruptible_sleep_on is interruptible, but for your task to
actually be awakened and your alarm handler to get some CPU,
it needs to be scheduled. If the BKL (big kernel lock) is
held, it won't be scheduled until it is released.

So, even though the semaphore that the "wake_up_interruptible()"
function called, has been enabled, not a lot will happen until the
kernel lock is released. The ISR code that executed
wake_up_interruptible() doesn't schedule. It just returns to your
interrupt handler.

Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
New book: http://www.AbominableFirebug.com/
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: Sleeping thread not receive signal until it wakes up

[Lee Revell]

On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
> Interruptible_sleep_on is interruptible, but for your task to
> actually be awakened and your alarm handler to get some CPU,
> it needs to be scheduled. If the BKL (big kernel lock) is
> held, it won't be scheduled until it is released.

You can schedule while holding the BKL and it will be dropped and reacquired.

Lee

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

On 3/7/07, Lee Revell <rlrevell@joe-job.com> wrote:
> On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
> > Interruptible_sleep_on is interruptible, but for your task to
> > actually be awakened and your alarm handler to get some CPU,
> > it needs to be scheduled. If the BKL (big kernel lock) is
> > held, it won't be scheduled until it is released.
>
> You can schedule while holding the BKL and it will be dropped and reacquired.
>
> Lee
>


My hardware is PowerPC architecture. Does it have any thing to do with
the kernel locking? Also, I saw CONFIG_LOCK_KERNEL,
CONFIG_PREEMPT_BKL ans CONFIG_SMP in the file
include/linux/smp_lock.h, or CONFIG_PREEMPT  in lib/kernel_lock.c and
I don't have any of these macro defined, would that be the reason. I
could not find where these option when running make menuconfig either.


Thanks,
LNgo

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
>
> >
> > On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
> >>
> >> On Wed, 7 Mar 2007, Luong Ngo wrote:
> >>
> >>> Hi all,
> >>>
> >>> I am having this problem. I have a process with 2 threads created. One
> >>> of the thread will keep calling IOCTL  to get information from the
> >>> kernel and will be blocked if there is no new information. If there is
> >>> information retured, the thread will be checked to see if any error
> >>> happens and trigger an action. Since we have no way to know if the
> >>> error is gone (Hardware provides no signal), so what we do is when
> >>> trigger an action for the error, we will set an timer using alarm()
> >>> and register a SIGALRM handler in the thread by using sigaction. After
> >>> setting the alarm, the thread will loop back and call IOCTL, which
> >>> could cause it to be put to sleep. The problem is the SIGALRM handler
> >>> does not receive the SIGALRM while the thread is being blocked by
> >>> IOCTL. And if we generated some event so that the IOCTL is returned
> >>> with new information, the SIGALRM handler is invoked right away.
> >>> However, as I read the manual, which says a thread/process should be
> >>> waken up even when it sleeps if there is a signal delivered to it. Am
> >>> I right?
> >>> One thing I don't know it mattters or not is that I am not using
> >>> sigwait to block the process and wait for signal because the thread
> >>> need to go back to the IOCTL call and be slept on that. So I used
> >>> sigaction to register the signal handler in hope that this handler wil
> >>> be invoked by the kernel when there is an SIGALRM delivered to the
> >>> thread.
> >>> Could anyone tell me if I did something wrong and what is the correct
> >>> way to achieve this task? I tried to avoid creating another thread
> >>> which will call sigwait and block until the IOCTL thread send it
> >>> explicitly a signal because I want to use timer.
> >>>
> >>>
> >>> Thank you in advance,
> >>> LNgo
> >>> -
> >>
> >> Later versions of the kernel lock the kernel when an ioctl() is
> >> entered. This means that if you sleep in the ioctl(), nothing
> >> will get scheduled.
> >>
> >> You can do the following (possibly unsafe) in your ioctl():
> >>
> >>     int locked = kernel_locked();
> >>
> >>     ......... code
> >>     ......... code
> >>
> >>     if(locked)  // Before sleeping section
> >>        unlock_kernel();
> >>     .......... sleeping code
> >>     if(locked)  // After sleeping section
> >>         lock_kernel();
> >>
> >>
> >>
> >> Cheers,
> >> Dick Johnson
> >> Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
> >> New book: http://www.AbominableFirebug.com/
> >> _
> >>
> >> Thank you.
> >>
> On Wed, 7 Mar 2007, Luong Ngo wrote:
>
> > Hi Dick,
> > Thanks for your response. In my ioctl in the kernel, I use
> > interruptible_sleep_on to sleep on a queue and will be wake up by the
> > the ISR routine when interrupt happens, so  isn't
> > interruptible_sleep_on supposed to be interruptable, from its name? I
> > am using kernel 2.6.14.
> >
> > Thanks again,
> > LNgo
>
> Please don't "top post," you need to put answers at the bottom.
>
> Interruptible_sleep_on is interruptible, but for your task to
> actually be awakened and your alarm handler to get some CPU,
> it needs to be scheduled. If the BKL (big kernel lock) is
> held, it won't be scheduled until it is released.
>
> So, even though the semaphore that the "wake_up_interruptible()"
> function called, has been enabled, not a lot will happen until the
> kernel lock is released. The ISR code that executed
> wake_up_interruptible() doesn't schedule. It just returns to your
> interrupt handler.
>
> Cheers,
> Dick Johnson
> Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
> New book: http://www.AbominableFirebug.com/
> _
> \x1a\x04
>
> ****************************************************************
> The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.
>
> Thank you.
>

I added the check if kernel_locked and then unlock_kernel if it is but
the signal handler is still not invoked.
Here is how my code is

static int my_ioctl(...)
{
      case GET_STH:



              spin_lock_irq(dev->lock);
               ...................
               locked = kernel_locked();
              if(locked)
                  unlock_kernel();

              spin_unlock_irq(dev->lock);
              interruptible_sleep_on(&qu);

             if(locked)
                 lock_kernel();
             spin_lock_irq(dev->lock);

             break;
}


Anything kernel configuration I need to be aware of to enable
preemption in kernel?


Thank you,
LNgo

Re: Sleeping thread not receive signal until it wakes up

[linux-os (Dick Johnson)]

On Wed, 7 Mar 2007, Luong Ngo wrote:

> On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
>>
>>>
>>> On 3/7/07, linux-os (Dick Johnson) <linux-os@analogic.com> wrote:
>>>>
>>>> On Wed, 7 Mar 2007, Luong Ngo wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> I am having this problem. I have a process with 2 threads created. One
>>>>> of the thread will keep calling IOCTL  to get information from the
>>>>> kernel and will be blocked if there is no new information. If there is
>>>>> information retured, the thread will be checked to see if any error
>>>>> happens and trigger an action. Since we have no way to know if the
>>>>> error is gone (Hardware provides no signal), so what we do is when
>>>>> trigger an action for the error, we will set an timer using alarm()
>>>>> and register a SIGALRM handler in the thread by using sigaction. After
>>>>> setting the alarm, the thread will loop back and call IOCTL, which
>>>>> could cause it to be put to sleep. The problem is the SIGALRM handler
>>>>> does not receive the SIGALRM while the thread is being blocked by
>>>>> IOCTL. And if we generated some event so that the IOCTL is returned
>>>>> with new information, the SIGALRM handler is invoked right away.
>>>>> However, as I read the manual, which says a thread/process should be
>>>>> waken up even when it sleeps if there is a signal delivered to it. Am
>>>>> I right?
>>>>> One thing I don't know it mattters or not is that I am not using
>>>>> sigwait to block the process and wait for signal because the thread
>>>>> need to go back to the IOCTL call and be slept on that. So I used
>>>>> sigaction to register the signal handler in hope that this handler wil
>>>>> be invoked by the kernel when there is an SIGALRM delivered to the
>>>>> thread.
>>>>> Could anyone tell me if I did something wrong and what is the correct
>>>>> way to achieve this task? I tried to avoid creating another thread
>>>>> which will call sigwait and block until the IOCTL thread send it
>>>>> explicitly a signal because I want to use timer.
>>>>>
>>>>>
>>>>> Thank you in advance,
>>>>> LNgo
>>>>> -
>>>>
>>>> Later versions of the kernel lock the kernel when an ioctl() is
>>>> entered. This means that if you sleep in the ioctl(), nothing
>>>> will get scheduled.
>>>>
>>>> You can do the following (possibly unsafe) in your ioctl():
>>>>
>>>>     int locked = kernel_locked();
>>>>
>>>>     ......... code
>>>>     ......... code
>>>>
>>>>     if(locked)  // Before sleeping section
>>>>        unlock_kernel();
>>>>     .......... sleeping code
>>>>     if(locked)  // After sleeping section
>>>>         lock_kernel();
>>>>
>>>>
>>>>
>>>> Cheers,
>>>> Dick Johnson
>>>> Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
>>>> New book: http://www.AbominableFirebug.com/
>>>> _
>>>>
>>>> Thank you.
>>>>
>> On Wed, 7 Mar 2007, Luong Ngo wrote:
>>
>>> Hi Dick,
>>> Thanks for your response. In my ioctl in the kernel, I use
>>> interruptible_sleep_on to sleep on a queue and will be wake up by the
>>> the ISR routine when interrupt happens, so  isn't
>>> interruptible_sleep_on supposed to be interruptable, from its name? I
>>> am using kernel 2.6.14.
>>>
>>> Thanks again,
>>> LNgo
>>
>> Please don't "top post," you need to put answers at the bottom.
>>
>> Interruptible_sleep_on is interruptible, but for your task to
>> actually be awakened and your alarm handler to get some CPU,
>> it needs to be scheduled. If the BKL (big kernel lock) is
>> held, it won't be scheduled until it is released.
>>
>> So, even though the semaphore that the "wake_up_interruptible()"
>> function called, has been enabled, not a lot will happen until the
>> kernel lock is released. The ISR code that executed
>> wake_up_interruptible() doesn't schedule. It just returns to your
>> interrupt handler.
>>
>> Cheers,
>> Dick Johnson
>> Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
>> New book: http://www.AbominableFirebug.com/
>> _
>> \x1a\x04
>>
>> ****************************************************************
>> The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.
>>
>> Thank you.
>>
>
> I added the check if kernel_locked and then unlock_kernel if it is but
> the signal handler is still not invoked.
> Here is how my code is
>
> static int my_ioctl(...)
> {
>      case GET_STH:
>
>
>
>              spin_lock_irq(dev->lock);
>               ...................
>               locked = kernel_locked();
>              if(locked)
>                  unlock_kernel();
>
>              spin_unlock_irq(dev->lock);
>              interruptible_sleep_on(&qu);
>
>             if(locked)
>                 lock_kernel();
>             spin_lock_irq(dev->lock);
>
>             break;
> }
>
>
> Anything kernel configuration I need to be aware of to enable
> preemption in kernel?
>
>
> Thank you,
> LNgo
>

First, in the ioctl, if you need spin-locks, you need to use
spin_lock_irqsave/spin_unlock/irqrestore. The ones that don't
save and restore are for the ISR where we know that the interrupts
are already off and don't intend to turn them on. Further, make
sure that you don't try to schedule() with the interrupts off.

interruptible_sleep_on(&qu);
                         ^______ Where is this?

This must be accessible both in the ISR and in the ioctl(). It
also needs to have been properly initialized when your module
was installed (see numerious code samples in the kernel).


Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.24 on an i686 machine (5592.71 BogoMips).
New book: http://www.AbominableFirebug.com/
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: Sleeping thread not receive signal until it wakes up

[Thomas Gleixner]

On Thu, 2007-03-08 at 08:01 -0500, linux-os (Dick Johnson) wrote:
> > Anything kernel configuration I need to be aware of to enable
> > preemption in kernel?
> >
> >
> > Thank you,
> > LNgo
> >
> 
> First, in the ioctl, if you need spin-locks, you need to use
> spin_lock_irqsave/spin_unlock/irqrestore. The ones that don't
> save and restore are for the ISR where we know that the interrupts
> are already off and don't intend to turn them on. Further, make

As usual completely wrong:

ioctl code is called with interrupts enabled. So if you need to protect
against interrupts it is completely correct to use spin_lock_irq /
spin_unlock_irq. There is nothing to save and nothing to restore.

> sure that you don't try to schedule() with the interrupts off.
> 
> interruptible_sleep_on(&qu);
>                          ^______ Where is this?

Not included in the pseudo code.

> This must be accessible both in the ISR and in the ioctl(). It
> also needs to have been properly initialized when your module
> was installed (see numerious code samples in the kernel).

Instead of blurbing about obvious things you should point Luong to the
real problems:

include/linux/wait.h:
/*
 * These are the old interfaces to sleep waiting for an event.
 * They are racy.  DO NOT use them, use the wait_event* interfaces above.
 * We plan to remove these interfaces during 2.7.
 */
extern void FASTCALL(sleep_on(wait_queue_head_t *q));
extern long FASTCALL(sleep_on_timeout(wait_queue_head_t *q,
                                      signed long timeout));
extern void FASTCALL(interruptible_sleep_on(wait_queue_head_t *q));
extern long FASTCALL(interruptible_sleep_on_timeout(wait_queue_head_t *q,
                                                    signed long timeout));

Luong, please use wait_event_interruptible() instead.

	spin_lock_irq(dev->lock);
	do_whatever_you_need_to_initialize(dev);
	dev->event_happened = 0;
	spin_unlock_irq(dev->lock);
	
	res = wait_event_interruptible(&dev->queue, dev->event_happened != 0);

Thanks,

	tglx

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

On 3/8/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 2007-03-08 at 08:01 -0500, linux-os (Dick Johnson) wrote:
> > > Anything kernel configuration I need to be aware of to enable
> > > preemption in kernel?
> > >
> > >
> > > Thank you,
> > > LNgo
> > >
> >
> > First, in the ioctl, if you need spin-locks, you need to use
> > spin_lock_irqsave/spin_unlock/irqrestore. The ones that don't
> > save and restore are for the ISR where we know that the interrupts
> > are already off and don't intend to turn them on. Further, make
>
> As usual completely wrong:
>
> ioctl code is called with interrupts enabled. So if you need to protect
> against interrupts it is completely correct to use spin_lock_irq /
> spin_unlock_irq. There is nothing to save and nothing to restore.
>
> > sure that you don't try to schedule() with the interrupts off.
> >
> > interruptible_sleep_on(&qu);
> >                          ^______ Where is this?
>
> Not included in the pseudo code.
>
> > This must be accessible both in the ISR and in the ioctl(). It
> > also needs to have been properly initialized when your module
> > was installed (see numerious code samples in the kernel).
>
> Instead of blurbing about obvious things you should point Luong to the
> real problems:
>
> include/linux/wait.h:
> /*
>  * These are the old interfaces to sleep waiting for an event.
>  * They are racy.  DO NOT use them, use the wait_event* interfaces above.
>  * We plan to remove these interfaces during 2.7.
>  */
> extern void FASTCALL(sleep_on(wait_queue_head_t *q));
> extern long FASTCALL(sleep_on_timeout(wait_queue_head_t *q,
>                                      signed long timeout));
> extern void FASTCALL(interruptible_sleep_on(wait_queue_head_t *q));
> extern long FASTCALL(interruptible_sleep_on_timeout(wait_queue_head_t *q,
>                                                    signed long timeout));
>
> Luong, please use wait_event_interruptible() instead.
>
>        spin_lock_irq(dev->lock);
>        do_whatever_you_need_to_initialize(dev);
>        dev->event_happened = 0;
>        spin_unlock_irq(dev->lock);
>
>        res = wait_event_interruptible(&dev->queue, dev->event_happened != 0);
>
> Thanks,
>
>        tglx
>
>
>

Hi Thomas and Dick,
I appreciate all the responses. They are very good information to me.
Actually, it wasn't me working on the driver but it's been there long
time. I thought I just need to add the signal and signal handling
part, not expecting it would lead me to the driver space.
Here is what I have in the driver. Maybe racing condition could happen
in scenario that the ioctl realease the lock but befor going to sleep,
the ISR is invoked and call waking up on the queue, hence the ioctl
will not be waken up since the wak up cal already executed. But I
believe in our system, this could be tolerant since the hardware would
keep raising interrupt if the abnormal condition still exists (Due to
the ioctl being blocked so user app nevers get a chance to service the
device). But is this the reason why my signal handler not get executed
at all? Theoretically, according to the Richard Stevens book, I think
the process should be waken up and received the signal even if it gets
blocked in the IOCTL call, am i right?

static irqreturn board_isr(int irq, void *dev_id, struct pt_regs* regs)
{
 spin_lock(&dev->lock);
   if (dev->irqMask & (1 << irqBit)) {
    // Set the interrupt event mask
    dev->irqEvent |= (1 << irqBit);

    // Disable this irq, it will be reenabled after processed by board task
    disable_irq(irq);
    // Wake up Board thread that calling IOCTL
    wake_up(&(dev->boardIRQWaitQueue));
  }
  spin_unlock(&dev->lock);

  return IRQ_HANDLED;

}

static int ats89_ioctl(struct inode *inode, struct file *file, u_int
cmd, u_long arg)
{

          switch(cmd){
           case GET_IRQ_CMD: {
            u32  regMask32;

           spin_lock_irq(dev->lock);
           while ((dev->irqMask & dev->irqEvent) == 0) {
                 // Sleep until board interrupt happens
                 spin_unlock_irq(dev->lock);
                 interruptible_sleep_on(&(dev->boardIRQWaitQueue));
                 if (uncond_wakeup) {
                     /* don't go back to loop */
                     break;
                 }
                 spin_lock_irq(dev->lock);
             }

            uncond_wakeup = 0;

             // Board interrupt happened
            regMask32 = dev->irqMask & dev->irqEvent;
             if(copy_to_user(&(((ATS89_IOCTL_S *)arg)->mask32),
&regMask32, sizeof(u32))) {
                 spin_unlock_irq(dev->lock);
                 return -EAGAIN;
             }

             // Clear the event mask
             dev->irqEvent = 0;
             spin_unlock_irq(dev->lock);
        }
        break;


           }
}


Could you tell me why the blocking call to ioctl(GET_IRQ_CMD) in the
user space would block the whole process/thread to receive an alarm
signal delievered to it?

By the way, in my kernel, the lock_kernel, unlock_kernel are defined
as nothing just a simple do while loop and return right away, the
kernel_locked always return 1. There is know CONFIG_KERNEL_PREEMPT or
the likes CONFIG flag in my config file.

Thank you very much

LNgo

Re: Sleeping thread not receive signal until it wakes up

[Sergey Vlasov]

On Thu, 8 Mar 2007 14:52:07 -0800 Luong Ngo wrote:

[...]
> static irqreturn board_isr(int irq, void *dev_id, struct pt_regs* regs)
> {
>  spin_lock(&dev->lock);
>    if (dev->irqMask & (1 << irqBit)) {
>     // Set the interrupt event mask
>     dev->irqEvent |= (1 << irqBit);
> 
>     // Disable this irq, it will be reenabled after processed by board task
>     disable_irq(irq);

I assume that your device does not support shared interrupts?  If it
does (and a PCI device is required to support them), you cannot use
disable_irq() here (and you need to check a register in the device to
find out if it really did generate an IRQ)...

>     // Wake up Board thread that calling IOCTL
>     wake_up(&(dev->boardIRQWaitQueue));
>   }
>   spin_unlock(&dev->lock);
> 
>   return IRQ_HANDLED;

...and return IRQ_NONE here if the IRQ is not from your device.

> 
> }
> 
> static int ats89_ioctl(struct inode *inode, struct file *file, u_int
> cmd, u_long arg)
> {
> 
>           switch(cmd){
>            case GET_IRQ_CMD: {
>             u32  regMask32;
> 
>            spin_lock_irq(dev->lock);
>            while ((dev->irqMask & dev->irqEvent) == 0) {
>                  // Sleep until board interrupt happens
>                  spin_unlock_irq(dev->lock);
>                  interruptible_sleep_on(&(dev->boardIRQWaitQueue));
>                  if (uncond_wakeup) {
>                      /* don't go back to loop */
>                      break;
>                  }
>                  spin_lock_irq(dev->lock);
>              }
> 
>             uncond_wakeup = 0;
> 
>              // Board interrupt happened
>             regMask32 = dev->irqMask & dev->irqEvent;
>              if(copy_to_user(&(((ATS89_IOCTL_S *)arg)->mask32),
> &regMask32, sizeof(u32))) {
>                  spin_unlock_irq(dev->lock);
>                  return -EAGAIN;
>              }
> 
>              // Clear the event mask
>              dev->irqEvent = 0;
>              spin_unlock_irq(dev->lock);
>         }
>         break;
> 
> 
>            }
> }

And this code is full of bugs:

 1) As you have been told already, interruptible_sleep_on() and
    sleep_on() functions are broken and should not be used (they are
    left in the kernel only to support some obsolete code).  Either
    use wait_event_interruptible() or work with wait queues directly
    (prepare_to_wait(), finish_wait(), ...).

 2) The code to handle pending signals is missing - you need to have
    this after wait_event_interruptible():

		if (signal_pending(current))
			return -ERESTARTSYS;

    (but be careful - you might need to clean up something before
    returning).

    This is what causes your problem - interruptible_sleep_on()
    returns if a signal is pending, but your code does not check for
    signals and therefore invokes interruptible_sleep_on() again; but
    if a signal is pending, interruptible_sleep_on() returns
    immediately, causing your driver to eat 100% CPU looping in kernel
    mode until some device event finally happens.

 3) If uncond_wakeup is set, you break out of the loop with dev->lock
    unlocked; however, if dev->irqEvent gets set, you exit the loop
    with dev->lock locked.  The subsequent code always unlocks
    dev->lock, so in the uncond_wakeup case you have double unlock.

 4) You are doing copy_to_user() while holding a spinlock - this is
    prohibited (as any other form of sleep inside a spinlock).

 5) The return code for the copy_to_user() failure is wrong - it
    should be -EFAULT (this is not a fatal bug, but an annoyance for
    users of your driver, who might get such nonstandard error codes
    while debugging their programs and wonder what is going on).

Re: Sleeping thread not receive signal until it wakes up

[Luong Ngo]

On 3/9/07, Sergey Vlasov <vsu@altlinux.ru> wrote:
> On Thu, 8 Mar 2007 14:52:07 -0800 Luong Ngo wrote:
>
> [...]
> > static irqreturn board_isr(int irq, void *dev_id, struct pt_regs* regs)
> > {
> >  spin_lock(&dev->lock);
> >    if (dev->irqMask & (1 << irqBit)) {
> >     // Set the interrupt event mask
> >     dev->irqEvent |= (1 << irqBit);
> >
> >     // Disable this irq, it will be reenabled after processed by board task
> >     disable_irq(irq);
>
> I assume that your device does not support shared interrupts?  If it
> does (and a PCI device is required to support them), you cannot use
> disable_irq() here (and you need to check a register in the device to
> find out if it really did generate an IRQ)...

  Yes, the device does not share interrupt.

> > static int ats89_ioctl(struct inode *inode, struct file *file, u_int
> > cmd, u_long arg)
> > {
> >
> >           switch(cmd){
> >            case GET_IRQ_CMD: {
> >             u32  regMask32;
> >
> >            spin_lock_irq(dev->lock);
> >            while ((dev->irqMask & dev->irqEvent) == 0) {
> >                  // Sleep until board interrupt happens
> >                  spin_unlock_irq(dev->lock);
> >                  interruptible_sleep_on(&(dev->boardIRQWaitQueue));
> >                  if (uncond_wakeup) {
> >                      /* don't go back to loop */
> >                      break;
> >                  }
> >                  spin_lock_irq(dev->lock);
> >              }
> >
> >             uncond_wakeup = 0;
> >
> >              // Board interrupt happened
> >             regMask32 = dev->irqMask & dev->irqEvent;
> >              if(copy_to_user(&(((ATS89_IOCTL_S *)arg)->mask32),
> > &regMask32, sizeof(u32))) {
> >                  spin_unlock_irq(dev->lock);
> >                  return -EAGAIN;
> >              }
> >
> >              // Clear the event mask
> >              dev->irqEvent = 0;
> >              spin_unlock_irq(dev->lock);
> >         }
> >         break;
> >
> >
> >            }
> > }
>
> And this code is full of bugs:
>
>  1) As you have been told already, interruptible_sleep_on() and
>    sleep_on() functions are broken and should not be used (they are
>    left in the kernel only to support some obsolete code).  Either
>    use wait_event_interruptible() or work with wait queues directly
>    (prepare_to_wait(), finish_wait(), ...).
>
     I agree.but as I said our hardware will repeatedly raising
interrupts until it's serviced, the missing wakeup call would be
repeated also, so this should still wake up the sleep_on call. But we
would change it definitely.


>  2) The code to handle pending signals is missing - you need to have
>    this after wait_event_interruptible():
>
>                if (signal_pending(current))
>                        return -ERESTARTSYS;
>
>    (but be careful - you might need to clean up something before
>    returning).
>
>    This is what causes your problem - interruptible_sleep_on()
>    returns if a signal is pending, but your code does not check for
>    signals and therefore invokes interruptible_sleep_on() again; but
>    if a signal is pending, interruptible_sleep_on() returns
>    immediately, causing your driver to eat 100% CPU looping in kernel
>    mode until some device event finally happens.
>
     As pointed out by Robert, I added the checking
                   if(signal_pending(current))
                       return -ERESTARTSYS;
right after the line interruptible_sleep_on , but I don't see any
difference yet.

>  3) If uncond_wakeup is set, you break out of the loop with dev->lock
>    unlocked; however, if dev->irqEvent gets set, you exit the loop
>    with dev->lock locked.  The subsequent code always unlocks
>    dev->lock, so in the uncond_wakeup case you have double unlock.
>
  Thanks for catching it

>  4) You are doing copy_to_user() while holding a spinlock - this is
>    prohibited (as any other form of sleep inside a spinlock).
>
     Thanks again. But may I ask if it is prohibited, how come it has
been running without any error?

>  5) The return code for the copy_to_user() failure is wrong - it
>    should be -EFAULT (this is not a fatal bug, but an annoyance for
>    users of your driver, who might get such nonstandard error codes
>    while debugging their programs and wonder what is going on).
>
    changed.


Thank you for your input.

-LNgo

Re: Sleeping thread not receive signal until it wakes up

[Jan Engelhardt]

On Mar 7 2007 08:19, linux-os (Dick Johnson) wrote:
>
>Later versions of the kernel lock the kernel when an ioctl() is
>entered. This means that if you sleep in the ioctl(), nothing
>will get scheduled.

Later versions of the kernel also have an ->unlocked_ioctl method,
which is probably better than below's approach [which is akin to
FreeBSD's DROP_GIANT/PICKUP_GIANT].

>You can do the following (possibly unsafe) in your ioctl():
>
>     int locked = kernel_locked();
>
>     ......... code
>     ......... code
>
>     if(locked)  // Before sleeping section
>        unlock_kernel();
>     .......... sleeping code
>     if(locked)  // After sleeping section
>         lock_kernel();
>
>

Jan
-- 

Re: Sleeping thread not receive signal until it wakes up

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

On Tue, 6 Mar 2007 21:31:37 -0800 Luong Ngo wrote:

> I am having this problem. I have a process with 2 threads created. One
> of the thread will keep calling IOCTL  to get information from the
> kernel and will be blocked if there is no new information. If there is
> information retured, the thread will be checked to see if any error
> happens and trigger an action. Since we have no way to know if the
> error is gone (Hardware provides no signal), so what we do is when
> trigger an action for the error, we will set an timer using alarm()
> and register a SIGALRM handler in the thread by using sigaction. After
> setting the alarm, the thread will loop back and call IOCTL, which
> could cause it to be put to sleep. The problem is the SIGALRM handler
> does not receive the SIGALRM while the thread is being blocked by
> IOCTL. And if we generated some event so that the IOCTL is returned
> with new information, the SIGALRM handler is invoked right away.
> However, as I read the manual, which says a thread/process should be
> waken up even when it sleeps if there is a signal delivered to it. Am
> I right?

Probably the ioctl handler of the driver you are calling does not use
interruptible sleeps, therefore it cannot be interrupted by a signal.
In this case you need to fix the driver to use interruptible sleeps
and return -ERESTARTSYS when some signal is pending.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]