lkml.org 
[lkml]   [2004]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] IPMI driver updates, part 1b
Corey Minyard <minyard@acm.org> wrote:
>
> >diff -puN net/ipmi/af_ipmi.c~af_ipmi-locking-fix net/ipmi/af_ipmi.c
> >--- 25/net/ipmi/af_ipmi.c~af_ipmi-locking-fix Tue Feb 24 16:56:36 2004
> >+++ 25-akpm/net/ipmi/af_ipmi.c Tue Feb 24 16:57:00 2004
> >@@ -336,6 +336,7 @@ static int ipmi_recvmsg(struct kiocb *io
> > }
> >
> > timeo = ipmi_wait_for_queue(i, timeo);
> >+ spin_lock_irqsave(&i->lock, flags);
> > }
> >
> > rcvmsg = list_entry(i->msg_list.next, struct ipmi_recv_msg, link);
> >
> >
> > which may or may not be correct.
> >
> Actually, I believe the code is correct, and your change will break it.
> This is in a "while (1)" loop, and the only way to get out of this loop
> is to return with the lock not held or to break out of the loop with the
> lock held (and later code will unlock it). Am I correct here?

With a little more context:

+ spin_unlock_irqrestore(&i->lock, flags);
+ if (!timeo) {
+ return -EAGAIN;
+ } else if (signal_pending (current)) {
+ dbg("Signal pending: %d", 1);
+ return -EINTR;
+ }
+
+ timeo = ipmi_wait_for_queue(i, timeo);
+ }
+
+ rcvmsg = list_entry(i->msg_list.next, struct ipmi_recv_msg, link);
+ list_del(&rcvmsg->link);
+ spin_unlock_irqrestore(&i->lock, flags);
See, there's a direct code path from one spin_unlock() to the other. And
ipmi_wait_for_queue() does not retake the lock.

-
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/

\
 
 \ /
  Last update: 2005-03-22 14:01    [W:0.170 / U:0.292 seconds]
©2003-2011 Jasper Spaans. Advertise on this site