From owner-freebsd-usb@FreeBSD.ORG  Mon Nov  1 08:29:16 2010
Return-Path: <owner-freebsd-usb@FreeBSD.ORG>
Delivered-To: freebsd-usb@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 7A3F2106566C;
	Mon,  1 Nov 2010 08:29:16 +0000 (UTC)
	(envelope-from hselasky@c2i.net)
Received: from swip.net (mailfe08.swip.net [212.247.154.225])
	by mx1.freebsd.org (Postfix) with ESMTP id 9885F8FC0A;
	Mon,  1 Nov 2010 08:29:15 +0000 (UTC)
X-Cloudmark-Score: 0.000000 []
X-Cloudmark-Analysis: v=1.1 cv=4dE6tNKVm/0afO7MQGRPv7y2YwMo4emTIiDjbh74onY=
	c=1 sm=1 a=47Qj4_x6mI4A:10 a=8nJEP1OIZ-IA:10 a=CL8lFSKtTFcA:10
	a=i9M/sDlu2rpZ9XS819oYzg==:17 a=Rr0R_xDqafomNP2zMC0A:9
	a=XEVlvgEUUPTXRNKKEikA:7 a=-5gveJ-xTQQGrhZ7HWK8Wl3sap8A:4
	a=wPNLvfGTeEIA:10 a=i9M/sDlu2rpZ9XS819oYzg==:117
Received: from [188.126.198.129] (account mc467741@c2i.net HELO
	laptop002.hselasky.homeunix.org)
	by mailfe08.swip.net (CommuniGate Pro SMTP 5.2.19)
	with ESMTPA id 42913675; Mon, 01 Nov 2010 09:29:13 +0100
From: Hans Petter Selasky <hselasky@c2i.net>
To: Weongyo Jeong <weongyo@freebsd.org>
Date: Mon, 1 Nov 2010 09:30:25 +0100
User-Agent: KMail/1.13.5 (FreeBSD/8.1-STABLE; KDE/4.4.5; amd64; ; )
References: <20101031224304.GB3918@weongyo>
In-Reply-To: <20101031224304.GB3918@weongyo>
X-Face: +~\`s("[*<S!NI[0M*"FaghA8S>|O,="7?X@L.elg*F"OA\I/3%^p8g?ab%RN'(
	=?iso-8859-1?q?=3B=5FIjlA=3A=0A=09hGE=2E=2EEw?=,
	=?iso-8859-1?q?XAQ*o=23=5C/M=7ESC=3DS1-f9=7BEzRfT=27=7CHhll5Q=5Dha5Bt-s=7Co?=
	=?iso-8859-1?q?TlKMusi=3A1e=5BwJl=7Dkd=7DGR=0A=09Z0adGx-x=5F0zGbZj=27e?=(Y[(UNle~)8CQWXW@:DX+9)_YlB[tIccCPN$7/L'
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Message-Id: <201011010930.26038.hselasky@c2i.net>
Cc: freebsd-usb@freebsd.org
Subject: Re: [CFR 2-3/n] removes uther dependency of axe(4)
X-BeenThere: freebsd-usb@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: FreeBSD support for USB <freebsd-usb.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-usb>,
	<mailto:freebsd-usb-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-usb>
List-Post: <mailto:freebsd-usb@freebsd.org>
List-Help: <mailto:freebsd-usb-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-usb>,
	<mailto:freebsd-usb-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Mon, 01 Nov 2010 08:29:16 -0000

On Sunday 31 October 2010 23:43:04 Weongyo Jeong wrote:
> +static void
> +axe_watchdog(void *arg)
> +{
> +       struct axe_softc *sc = arg;
> +       struct ifnet *ifp = sc->sc_ifp;
> +
> +       if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0)
> +               return;
> +

Hi,

Please explain what is wrong with the existing code regarding code 
synchronisation. Your patch is very big and is likely to introduce problems.

I oppose the introduction of SX-locks. Please explain why you think SX-locks 
are better than the USB process taskqueue.

Are you absolutely sure that all the IOCTL's that are called are allowed to 
block in the way you have programmed?

The checks in xxx_watchdog() are not good enough. axe_tick() will execute 
synchronous USB functions, which sleep for many hundreds of microseconds. You 
should add this check before the sleepout_reset() too, and is this code called 
with any lock locked? I.E. Are you doing the clearing of IFF_DRV_RUNNING 
atomic to testing this flag? Else the result can be random?

--HPS