[alsa-devel] snd_mixart_send_msg / snd_mixart_send_msg_wait_notif
Hi.
While looking through ALSA for schedule_timeout() calls, I ran into:
int snd_mixart_send_msg(...) {
[ ... ]
set_current_state(TASK_UNINTERRUPTIBLE); add_wait_queue(&mgr->msg_sleep, &wait); spin_unlock_irq(&mgr->msg_lock); timeout = schedule_timeout(MSG_TIMEOUT_JIFFIES); remove_wait_queue(&mgr->msg_sleep, &wait);
if (! timeout) { /* error - no ack */ mutex_unlock(&mgr->msg_mutex); snd_printk(KERN_ERR "error: no reponse on msg %x\n", msg_frame); return -EIO; }
[ ... ]
}
and the same in snd_mixart_send_msg_wait_notif().
I believe there to be something wrong with this code. A schedule_timeout() in TASK_UNINTERRUPTIBLE is always going to return 0. Didn't you intend to use a wait_event_timeout() or something like that? Puzzled, since you err out on !timeout, and it seems every run through this would end up there.
Rene.
On 09/18/2007 01:54 PM, Rene Herman wrote:
People just _make_ me reply to myself:
alsa@digigram.com SMTP error from remote mailer after RCPT TO:alsa@digigram.com: host mail.digigram.com [213.30.172.230]: 550 5.1.1 alsa@digigram.com: Recipient address rejected: User unknown in relay recipient table
While looking through ALSA for schedule_timeout() calls, I ran into:
int snd_mixart_send_msg(...) {
[ ... ] set_current_state(TASK_UNINTERRUPTIBLE); add_wait_queue(&mgr->msg_sleep, &wait); spin_unlock_irq(&mgr->msg_lock); timeout = schedule_timeout(MSG_TIMEOUT_JIFFIES); remove_wait_queue(&mgr->msg_sleep, &wait); if (! timeout) { /* error - no ack */ mutex_unlock(&mgr->msg_mutex); snd_printk(KERN_ERR "error: no reponse on msg %x\n",msg_frame); return -EIO; }
[ ... ]}
and the same in snd_mixart_send_msg_wait_notif().
I believe there to be something wrong with this code. A schedule_timeout() in TASK_UNINTERRUPTIBLE is always going to return 0. Didn't you intend to use a wait_event_timeout() or something like that? Puzzled, since you err out on !timeout, and it seems every run through this would end up there.
Rene.
At Tue, 18 Sep 2007 13:54:23 +0200, Rene Herman wrote:
Hi.
While looking through ALSA for schedule_timeout() calls, I ran into:
int snd_mixart_send_msg(...) {
[ ... ]
set_current_state(TASK_UNINTERRUPTIBLE); add_wait_queue(&mgr->msg_sleep, &wait); spin_unlock_irq(&mgr->msg_lock); timeout = schedule_timeout(MSG_TIMEOUT_JIFFIES); remove_wait_queue(&mgr->msg_sleep, &wait); if (! timeout) { /* error - no ack */ mutex_unlock(&mgr->msg_mutex); snd_printk(KERN_ERR "error: no reponse on msg %x\n",msg_frame); return -EIO; }
[ ... ]
}
and the same in snd_mixart_send_msg_wait_notif().
I believe there to be something wrong with this code. A schedule_timeout() in TASK_UNINTERRUPTIBLE is always going to return 0.
No, it returns the time left at the point it's woken up. *_uninterruptible() ignores signals but not wake_up() itself (otherwise it makes no sense). The description in the kernel comment is misleading indeed.
Takashi
On 09/18/2007 02:40 PM, Takashi Iwai wrote:
At Tue, 18 Sep 2007 13:54:23 +0200, Rene Herman wrote:
Hi.
While looking through ALSA for schedule_timeout() calls, I ran into:
int snd_mixart_send_msg(...) {
[ ... ]
set_current_state(TASK_UNINTERRUPTIBLE); add_wait_queue(&mgr->msg_sleep, &wait); spin_unlock_irq(&mgr->msg_lock); timeout = schedule_timeout(MSG_TIMEOUT_JIFFIES); remove_wait_queue(&mgr->msg_sleep, &wait); if (! timeout) { /* error - no ack */ mutex_unlock(&mgr->msg_mutex); snd_printk(KERN_ERR "error: no reponse on msg %x\n",msg_frame); return -EIO; }
[ ... ]
}
and the same in snd_mixart_send_msg_wait_notif().
I believe there to be something wrong with this code. A schedule_timeout() in TASK_UNINTERRUPTIBLE is always going to return 0.
No, it returns the time left at the point it's woken up. *_uninterruptible() ignores signals but not wake_up() itself (otherwise it makes no sense). The description in the kernel comment is misleading indeed.
And the wake_up() is done from interrupt here -- yes, okay, I understand now. Thanks.
(send email to digigram asking what happened to that alsa@ address by the way).
Rene.
participants (2)
-
Rene Herman -
Takashi Iwai