NetBSD Problem Report #48908

From www@NetBSD.org  Sat Jun 14 14:12:24 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C737FA654D
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 14 Jun 2014 14:12:24 +0000 (UTC)
Message-Id: <20140614141223.71612A6555@mollari.NetBSD.org>
Date: Sat, 14 Jun 2014 14:12:23 +0000 (UTC)
From: oshima-ya@yagoto-urayama.jp
Reply-To: oshima-ya@yagoto-urayama.jp
To: gnats-bugs@NetBSD.org
Subject: Cannot open /dev/uhid? when another report id at the same uhidev was already opened.
X-Send-Pr-Version: www-1.0

>Number:         48908
>Category:       kern
>Synopsis:       Cannot open /dev/uhid? when another report id at the same uhidev was already opened.
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jun 14 14:15:00 +0000 2014
>Closed-Date:    Wed Jun 18 03:08:53 +0000 2014
>Last-Modified:  Wed Jun 18 03:08:53 +0000 2014
>Originator:     Yasushi Oshima
>Release:        NetBSD 6.99.43
>Organization:
>Environment:
NetBSD s205 6.99.43 NetBSD 6.99.43 (GENERIC) #1: Sat Jun 14 13:22:53 JST 2014  oshima@sweety:/export/current/daily/20140611/obj/amd64/sys/arch/amd64/compile/GENERIC amd64
>Description:
I have the Wacom USB tablet.

When putting this into a usb-port on NetBSD/amd64 6.99.43,

uhidev0 at uhub2 port 1 configuration 1 interface 0
uhidev0: WACOM CTE-440-U V4.0-3, rev 1.10/4.03, addr 2, iclass 3/1
uhidev0: 3 report ids
ums0 at uhidev0 reportid 1: 3 buttons and Z dir
wsmouse1 at ums0 mux 0
uhid0 at uhidev0 reportid 2: input=7, output=0, feature=2
uhid1 at uhidev0 reportid 3: input=0, output=0, feature=2

This tablet has one mouse(report id 1:ums0) and one stylus(report id 2:uhid0) on the same /dev/uhidev0 device.

When I run 'startx', the ums0 will be opened by wsmux a.k.a. /dev/wsmouse. After that, when I run a 'usbhidctl -f /dev/uhid0 -a', it fails as followings:

# usbhidctl -f /dev/uhid0 -a -l
usbhidctl: /dev/uhid0: Input/output error

When run this on NetBSD 6.1.4, this works well:
# usbhidctl -f /dev/uhid0 -a -l
Digitizer.Digitizer.Touch=0
Digitizer.Digitizer.Barrel_Switch=0
Digitizer.Digitizer.Barrel_Switch=0
Digitizer.Digitizer.Invert=0
Digitizer.Digitizer.Transducer_Index=0
Digitizer.Digitizer.In_Range=1
Digitizer.Digitizer.X=3053
Digitizer.Digitizer.Y=4230
Digitizer.Digitizer.Tip_Pressure=0
:

This problem occurs after following changes at 2012/06/10 on -current:

http://mail-index.netbsd.org/source-changes/2012/06/10/msg034887.html

>How-To-Repeat:
1) Put HID device(ums(4), ukbd(4), or uhid(4)) which has two or more report ids on one uhidev(4).
2) Open one device. For example: startx to open /dev/wsmouse (the case of ums(4)).
3) Open another report-id device on the same uhidev(4). For example, open /dev/uhid0 by usbhidctl -f /dev/uhid0 -a -l.

>Fix:
The uhidev(4) device has one or more HID devices and each HID device should be opened separately. However, the current's uhidev_open() will be able to call only once, the second call will fail return with EIO.

This will count the open number and the second or later open will be  successful, same as netbsd-6.

RCS file: /cvsroot/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.59
diff -u -r1.59 uhidev.c
--- uhidev.c    26 Dec 2013 15:32:48 -0000      1.59
+++ uhidev.c    9 Jun 2014 16:02:31 -0000
@@ -545,6 +545,10 @@
                return (EBUSY);
        }
        scd->sc_state |= UHIDEV_OPEN;
+       if (sc->sc_refcnt++) {
+               mutex_exit(&sc->sc_lock);
+               return (0);
+       }
        mutex_exit(&sc->sc_lock);

        if (sc->sc_isize == 0)
@@ -604,6 +608,7 @@
        free(sc->sc_ibuf, M_USBDEV);
        mutex_enter(&sc->sc_lock);
        scd->sc_state &= ~UHIDEV_OPEN;
+       sc->sc_refcnt = 0;
        sc->sc_ibuf = NULL;
        sc->sc_ipipe = NULL;
        sc->sc_opipe = NULL;
@@ -623,6 +628,10 @@
                return;
        }
        scd->sc_state &= ~UHIDEV_OPEN;
+       if (--sc->sc_refcnt) {
+               mutex_exit(&sc->sc_lock);
+               return;
+       }
        mutex_exit(&sc->sc_lock);

        DPRINTF(("uhidev_close: close pipe\n"));
Index: uhidev.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uhidev.h,v
retrieving revision 1.14
diff -u -r1.14 uhidev.h
--- uhidev.h    26 Sep 2013 07:25:31 -0000      1.14
+++ uhidev.h    9 Jun 2014 16:02:31 -0000
@@ -53,6 +53,7 @@
        u_int sc_nrepid;
        device_t *sc_subdevs;

+       int sc_refcnt;
        u_char sc_dying;

        kmutex_t sc_lock;               /* protects writes to sc_state */

>Release-Note:

>Audit-Trail:
From: matthew green <mrg@eterna.com.au>
To: oshima-ya@yagoto-urayama.jp
Cc: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org,
    gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: re: kern/48908: Cannot open /dev/uhid? when another report id at the same uhidev was already opened.
Date: Sun, 15 Jun 2014 08:17:08 +1000

 would you please try this patch, see if it helps?  (thank's to skrll@
 for pointing this out to me.)  i've compile-tested this only, i'm not
 in a good position to test it against my own systems right now.

 thanks.


 .mrg.


 Index: uhidev.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/uhidev.c,v
 retrieving revision 1.59
 diff -p -r1.59 uhidev.c
 *** uhidev.c	26 Dec 2013 15:32:48 -0000	1.59
 --- uhidev.c	14 Jun 2014 22:15:50 -0000
 *************** uhidev_open(struct uhidev *scd)
 *** 537,543 ****
   	usbd_status err;
   	int error;

 ! 	DPRINTF(("uhidev_open: open pipe, state=%d\n", scd->sc_state));

   	mutex_enter(&sc->sc_lock);
   	if (scd->sc_state & UHIDEV_OPEN) {
 --- 537,544 ----
   	usbd_status err;
   	int error;

 ! 	DPRINTF(("uhidev_open: open pipe, state=%d refcnt=%d\n",
 ! 		 scd->sc_state, sc->sc_refcnt));

   	mutex_enter(&sc->sc_lock);
   	if (scd->sc_state & UHIDEV_OPEN) {
 *************** uhidev_open(struct uhidev *scd)
 *** 545,550 ****
 --- 546,553 ----
   		return (EBUSY);
   	}
   	scd->sc_state |= UHIDEV_OPEN;
 + 	if (sc->sc_refcnt++)
 + 		return (0);
   	mutex_exit(&sc->sc_lock);

   	if (sc->sc_isize == 0)
 *************** out1:
 *** 604,609 ****
 --- 607,613 ----
   	free(sc->sc_ibuf, M_USBDEV);
   	mutex_enter(&sc->sc_lock);
   	scd->sc_state &= ~UHIDEV_OPEN;
 + 	sc->sc_refcnt = 0;
   	sc->sc_ibuf = NULL;
   	sc->sc_ipipe = NULL;
   	sc->sc_opipe = NULL;
 *************** uhidev_close(struct uhidev *scd)
 *** 623,628 ****
 --- 627,634 ----
   		return;
   	}
   	scd->sc_state &= ~UHIDEV_OPEN;
 + 	if (--sc->sc_refcnt)
 + 		return;
   	mutex_exit(&sc->sc_lock);

   	DPRINTF(("uhidev_close: close pipe\n"));
 Index: uhidev.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/uhidev.h,v
 retrieving revision 1.14
 diff -p -r1.14 uhidev.h
 *** uhidev.h	26 Sep 2013 07:25:31 -0000	1.14
 --- uhidev.h	14 Jun 2014 22:15:50 -0000
 *************** struct uhidev_softc {
 *** 53,58 ****
 --- 53,59 ----
   	u_int sc_nrepid;
   	device_t *sc_subdevs;

 + 	int sc_refcnt;
   	u_char sc_dying;

   	kmutex_t sc_lock;		/* protects writes to sc_state */

From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org,
    gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
    oshima-ya@yagoto-urayama.jp
Cc: 
Subject: re: kern/48908: Cannot open /dev/uhid? when another report id at the same uhidev was already opened.
Date: Sun, 15 Jun 2014 09:45:15 +1000

 i see you sent a patch along with the original PR, and the
 version i sent has an error.  (thanks tsutsui.)  this is
 what happens when you don't read the full PR..

 ignore my patch, except maybe the first chunk..


 .mrg.

From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48908 CVS commit: src/sys/dev/usb
Date: Tue, 17 Jun 2014 09:35:46 +0000

 Module Name:	src
 Committed By:	skrll
 Date:		Tue Jun 17 09:35:46 UTC 2014

 Modified Files:
 	src/sys/dev/usb: uhidev.c uhidev.h

 Log Message:
 PR/48908: Cannot open /dev/uhid? when another report id at the same uhidev
 was already opened.

 Restore the sc reference counting lost in usbmp merge.


 To generate a diff of this commit:
 cvs rdiff -u -r1.59 -r1.60 src/sys/dev/usb/uhidev.c
 cvs rdiff -u -r1.14 -r1.15 src/sys/dev/usb/uhidev.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Tue, 17 Jun 2014 09:41:12 +0000
State-Changed-Why:
OK to close?


From: Yasushi Oshima <oshima-ya@yagoto-urayama.jp>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/48908 (Cannot open /dev/uhid? when another report id at
 the same uhidev was already opened.)
Date: Wed, 18 Jun 2014 08:26:06 +0900 (JST)

 > Synopsis: Cannot open /dev/uhid? when another report id at the same uhidev was already opened.
 > 
 > State-Changed-From-To: open->feedback
 > State-Changed-By: skrll@NetBSD.org
 > State-Changed-When: Tue, 17 Jun 2014 09:41:12 +0000
 > State-Changed-Why:
 > OK to close?
 > 

 It works well after update.

 Thanks a lot.
 Please close PR.

 -- 
 Yasushi Oshima

State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Wed, 18 Jun 2014 03:08:53 +0000
State-Changed-Why:
confirmed fixed, thanks


>Unformatted:

Home	PR Database Search