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.021 / U:89.124 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site