From owner-freebsd-current@FreeBSD.ORG Fri Sep 10 20:28:57 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 007A11065672; Fri, 10 Sep 2010 20:28:57 +0000 (UTC) (envelope-from weongyo.jeong@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 5AFAE8FC08; Fri, 10 Sep 2010 20:28:55 +0000 (UTC) Received: by wyb33 with SMTP id 33so3763879wyb.13 for ; Fri, 10 Sep 2010 13:28:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:received:from:date:to:cc :subject:message-id:reply-to:mail-followup-to:references :mime-version:content-type:content-disposition:in-reply-to :user-agent:organization:x-operation-sytem; bh=pz8YnX9Pm9ap+CR/qDC/J8aXBIdqAnWcxDBo3dcCug4=; b=Vh/C83slj5Biqw58BHHaR5mmlaklipck8mIZYuCcCsKdsQEDBCXXdkalQGa6wB8EMd /jqN0hv1yJCE+uFMUFdzpZe3/0j8PWn75idQJQbfyojBCOYcA23rcqrgoUr0BloEfDWA 0B95RQheyNL47+skakJ5cCf6s8zfj32VhS3Iw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:date:to:cc:subject:message-id:reply-to:mail-followup-to :references:mime-version:content-type:content-disposition :in-reply-to:user-agent:organization:x-operation-sytem; b=ZCYDjiHmfTmSfgxyuVD1a7PlHpOZkmLfRMI/kA655e28c9EAWW+m7gM3Sj+5kW0Fo7 iRSUWSAYp/J0nnJ1uLAgHSsJp84iLctwl3wpMyNSGJNqROBwexxZRGAUibAefljzbNec KTIMLfcYuaiwjRe9xBvdYQu5kD6VSwn4jqhCg= Received: by 10.227.141.77 with SMTP id l13mr339487wbu.77.1284150534216; Fri, 10 Sep 2010 13:28:54 -0700 (PDT) Received: from weongyo ([174.35.1.224]) by mx.google.com with ESMTPS id b23sm2586922wbb.4.2010.09.10.13.28.51 (version=SSLv3 cipher=RC4-MD5); Fri, 10 Sep 2010 13:28:52 -0700 (PDT) Received: by weongyo (sSMTP sendmail emulation); Fri, 10 Sep 2010 13:28:58 -0700 From: Weongyo Jeong Date: Fri, 10 Sep 2010 13:28:58 -0700 To: John Baldwin Message-ID: <20100910202858.GI1328@weongyo> Mail-Followup-To: John Baldwin , freebsd-current@freebsd.org References: <20100908201419.GF1328@weongyo> <201009090936.19412.jhb@freebsd.org> <20100909174146.GG1328@weongyo> <201009091432.52066.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201009091432.52066.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i Organization: CDNetworks. X-Operation-Sytem: FreeBSD Cc: freebsd-current@freebsd.org Subject: Re: about in_multi_mtx @ netinet/in_mcast.c:1095 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Weongyo Jeong List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Sep 2010 20:28:57 -0000 On Thu, Sep 09, 2010 at 02:32:52PM -0400, John Baldwin wrote: > On Thursday, September 09, 2010 1:41:46 pm Weongyo Jeong wrote: > > On Thu, Sep 09, 2010 at 09:36:19AM -0400, John Baldwin wrote: > > > On Wednesday, September 08, 2010 4:14:19 pm Weongyo Jeong wrote: > > > > Hello, > > > > > > > > I have a question about IN_MULTI_LOCK() because it uses MTX_DEF flag > > > > when it's initialized so I always encounters the following LOR > > > > > > > > lock order reversal: (sleepable after non-sleepable) > > > > 1st 0xffffffff80d0b560 in_multi_mtx (in_multi_mtx) @ > > > netinet/in_mcast.c:1095 > > > > 2nd 0xffffff00014e3850 USB device SX lock (USB device SX lock) @ > > > > /usr/home/freebsd_usb/sys/modules/usb/usb/../../../dev/usb/usb_request.c:441 > > > > KDB: stack backtrace: > > > > db_trace_self_wrapper() at db_trace_self_wrapper+0x2a > > > > _witness_debugger() at _witness_debugger+0x2e > > > > witness_checkorder() at witness_checkorder+0x807 > > > > _sx_xlock() at _sx_xlock+0x55 > > > > usbd_do_request_flags() at usbd_do_request_flags+0xe5 > > > > axe_cmd() at axe_cmd+0xc7 > > > > axe_setmulti_locked() at axe_setmulti_locked+0x70 > > > > axe_setmulti() at axe_setmulti+0x3e > > > > axe_ioctl() at axe_ioctl+0x132 > > > > if_addmulti() at if_addmulti+0x19b > > > > in_joingroup_locked() at in_joingroup_locked+0x1bc > > > > in_joingroup() at in_joingroup+0x52 > > > > in_control() at in_control+0x1144 > > > > ifioctl() at ifioctl+0x1118 > > > > kern_ioctl() at kern_ioctl+0xbe > > > > ioctl() at ioctl+0xfd > > > > syscallenter() at syscallenter+0x1aa > > > > syscall() at syscall+0x4c > > > > Xfast_syscall() at Xfast_syscall+0xe2 > > > > > > > > when I uses the following code at driver's ioctl routine: > > > > > > > > case SIOCADDMULTI: > > > > case SIOCDELMULTI: > > > > axe_setmulti(sc, 0); > > > > break; > > > > > > > > It means that USB driver always should defer SIOCADDMULTI / > > > > SIOCDELMULTI handling to the other process context to avoid LOR. > > > > > > > > My question is that is it safe if the multicasting operations for USB > > > > device happens without IN_MULTI_LOCK? Or is there any race cases if the > > > > task is deferred? > > > > > > Why is USB using an sx lock instead of a mutex? > > > > Frankly speaking I also don't know why hps@ uses sx lock. That is one > > of things I'd like to change it. > > > > Just looking the comment at usb_request.c@441: > > > > /* > > * Grab the default sx-lock so that serialisation > > * is achieved when multiple threads are involved: > > */ > > sx_xlock(&udev->ctrl_sx); > > > > I think he might want to hold the lock even if the thread is going into > > sleep. It might be for serialization. > > > > However even if we succeed to change the lock from sx to mutex, it's > > hard to avoid the requests going into the sleep. It means USB stack > > should call like below: > > > > mtx_sleep(chan, IN_MULTI_LOCK, ...); > > > > to avoid the kernel's complain (would be `sleep with holding > > non-sleepable lock'). > > > > What I'd like to say is that the sleeping is big problem if mutex is > > used that it'd be worse when multiple mutex locks are used. > > > > So I'm looking for a fundamental solution to solve this problem. > > Welcomes any ideas. > > It is probably fine to just schedule a task to do the actual work of > axe_setmulti(). I think you do not need to lock IN_MULTI_LOCK yourself in > your task handler as long as your handler holds the appropriate lock > (if_maddr_rlock() IIRC) when walking the interface's multicast address list. OK. I'll try to use a task handler for this case. One thing, just for curious. Why the lower layer (in this case it might be netinet/in_mcast.c) calls ioctl interface with holding IN_MULTI_LOCK? If it calls ioctl without holding the lock, all drivers (specially all USB drivers) which handles SIOCADDMULTI and SIOCDELMULTI don't need to implement their own taskqueue or process context. It looks to me that calling ioctl interface with holding IN_MULTI_LOCK is useless if the drivers hold if_maddr_rlock(ifp) lock properly though I could miss something important. regards, Weongyo Jeong