svn commit: r246397 - head/sys/dev/syscons

[Hans Petter Selasky]

On Wednesday 06 February 2013 16:36:12 Ian Lepore wrote:
> On Wed, 2013-02-06 at 11:16 +0000, Hans Petter Selasky wrote:
> > Author: hselasky
> > Date: Wed Feb  6 11:16:18 2013
> > New Revision: 246397
> > URL: http://svnweb.freebsd.org/changeset/base/246397
> > 
> > Log:
> >   Make sure that all mouse buttons are released when clients
> >   using /dev/consolectl close. This fixes a problem where if
> >   a USB mouse is detached while a button is pressed, that
> >   button is never released.
> >   
> >   MFC after:	1 week
> > 
> > Modified:
> >   head/sys/dev/syscons/syscons.c
> > 
> > Modified: head/sys/dev/syscons/syscons.c
> > =========================================================================
> > ===== --- head/sys/dev/syscons/syscons.c	Wed Feb  6 11:10:41
> > 2013	(r246396) +++ head/sys/dev/syscons/syscons.c	Wed Feb  6 11:16:18
> > 2013	(r246397) @@ -253,11 +253,13 @@ static struct ttydevsw sc_ttydevsw
> > = {
> > 
> >  };
> >  
> >  static d_ioctl_t	consolectl_ioctl;
> > 
> > +static d_close_t	consolectl_close;
> > 
> >  static struct cdevsw consolectl_devsw = {
> >  
> >  	.d_version	= D_VERSION,
> > 
> > -	.d_flags	= D_NEEDGIANT,
> > +	.d_flags	= D_NEEDGIANT | D_TRACKCLOSE,
> > 
> >  	.d_ioctl	= consolectl_ioctl,
> > 
> > +	.d_close	= consolectl_close,
> > 
> >  	.d_name		= "consolectl",
> >  
> >  };
> > 
> > @@ -1561,6 +1563,23 @@ consolectl_ioctl(struct cdev *dev, u_lon
> > 
> >  	return sctty_ioctl(dev->si_drv1, cmd, data, td);
> >  
> >  }
> > 
> > +static int
> > +consolectl_close(struct cdev *dev, int flags, int mode, struct thread
> > *td) +{
> > +#ifndef SC_NO_SYSMOUSE
> > +	mouse_info_t info;
> > +	memset(&info, 0, sizeof(info));
> > +	info.operation = MOUSE_ACTION;
> > +
> > +	/*
> > +	 * Make sure all buttons are released when moused and other
> > +	 * console daemons exit, so that no buttons are left pressed.
> > +	 */
> > +	(void) sctty_ioctl(dev->si_drv1, CONS_MOUSECTL, (caddr_t)&info, td);
> > +#endif
> > +	return (0);
> > +}
> > +
> > 
> >  static void
> >  sc_cnprobe(struct consdev *cp)
> >  {
> 

Hi Ian,

> I think the D_TRACKCLOSE flag is not what you want here.  Based on my
> (admittedly still vague) understanding of it, that flag is almost never
> the right thing for a driver to use.
>
> If you need that action to be taken when the last open instance of the
> driver is closed, 

I don't want to wait until the last instance is closed. There are multiple 
instances of moused, for example. Each instance has a handle on 
/dev/consolectl . When one moused instance exits it closes this handle and I 
want to flush the mouse buttons. I tought about adding a cdevpriv structure to 
keep track of the mouse buttons on this file handle, though that seems to be 
overkill.

> that's what you get without D_TRACKCLOSE.  If you need
> it to be called as many times as the open routine was called... well,
> that's the the confusing thing.  It's not clear to me that there's any
> way to get that effect, but the cdevpriv destructor may be the closest
> thing available.

D_TRACKCLOSE was added to get the close call through each time.

> 
> I think the essential problem is that open and close are not strictly
> paired. 

That only happens when you destroy the device and the consolectl device is not 
destroyed. Else they will be paired.

> For example, a forked child inherits open descriptors without
> open() calls happening, and revoke() is a way to destroy references
> without a close() call.  But the details of all this are murky to me,
> and the most-enlightening mailing list posts you can find on it are full
> of phrases like "vref counters for devfs nodes" and "vnode is reclaimed"
> that make the discussion difficult to follow if you're not intimate with
> the internals of the vm system.

Right, what other clients of /dev/consolectl do we have? And do some of them 
pass on the FD like you describe?

--HPS