Messages in this thread | | | From | Jarod Wilson <> | Subject | Re: [PATCH 08/18] lirc driver for the Soundgraph IMON IR Receivers | Date | Wed, 24 Sep 2008 16:21:38 -0400 |
| |
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/
| |