Re: [patch] turning devctl into a "multiple openable" device

On Wed, Nov 30, 2011 at 05:46:36PM +0200, Kostik Belousov wrote:
> On Wed, Nov 30, 2011 at 10:05:11AM -0500, John Baldwin wrote:
> > On Wednesday, November 30, 2011 7:43:20 am Baptiste Daroussin wrote:
> > > Hi all,
> > > 
> > > With the help of cognet, I wrote a patch to turn devctl into a multiple openable
> > > device, that mean that it will allow to open /dev/devctl in multiple programs,
> > > for example hald and everythings that want to receive notification from the
> > > device won't need to depend on haveing devd running.
> > > 
> > > here is the patch: 
> > > http://people.freebsd.org/~bapt/devctl_multi_open.diff
> > 
> > Shouldn't devctl_queue_data_f() use the requested malloc() flags instead of
> > hardcoding M_NOWAIT?
> This is an obvious fallback of holding mutex around the call to
> per_devctl_queue_data_f(), which caused the author a trouble to use
> M_WAITOK.
> 
> Having n readers causes the patch to queue each message n times, that looks
> like a waste.
> 

Queuing the message only one time would require to somehow keep a state, to
know which thread read which message, and figuring out when to free a message
can be an headache. Given I don't think they'll be a lot of readers, I'm not
sure it's worth the trouble.

> I wonder why the waiting_threads stuff is needed at all. The cv could
> be woken up unconditionally everytime. What is the reason for the cv_wait
> call in cdevpriv data destructor ? You cannot have a thread doing e.g.
> read on the file descriptor while destructor is run.
> 

What will prevent you from having a thread stuck in read(), while an another 
one close() the fd ?


Regards,


Olivier