lkml.org 
[lkml]   [2008]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 08/18] lirc driver for the Soundgraph IMON IR Receivers
    Date
    On Monday 22 September 2008 17:47:56 Jarod Wilson wrote:
    > > > +/**
    > > > + * Called by lirc_dev when the application opens /dev/lirc
    > > > + */
    > > > +static int ir_open(void *data)
    > > > +{
    > > > +   int retval = SUCCESS;
    > > > +   struct imon_context *context;
    > > > +
    > > > +   /* prevent races with disconnect */
    > > > +   down(&disconnect_sem);
    > > > +
    > > > +   context = (struct imon_context *) data;
    > > > +
    > > > +   LOCK_CONTEXT;
    > > > +
    > > > +   if (context->ir_isopen) {
    > > > +           err("%s: IR port is already open", __func__);
    > > > +           retval = -EBUSY;
    > > > +           goto exit;
    > > > +   }
    > >
    > > I wonder if the single-open semantics are really doing what the author
    > > intended?  It is unsufficient to prevent concurrent calls elsewhere.
    >
    > I think we're good to go with disconnect_sem being converted to an actual
    > mutex, no?

    No. The lock here is worthless, as is the check for context->ir_isopen, as
    this function *only* gets called with the driver_lock held in lirc_dev, and
    lirc_dev won't call this if the device is already open. I've simply dropped
    the context lock, as its not needed at all (and in fact, triggered lockdep
    spew).

    --
    Jarod Wilson
    jarod@redhat.com

    --
    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: 2008-09-24 22:25    [from the cache]
    ©2003-2014 Jasper Spaans. Advertise on this site