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-2011 Jasper Spaans