From owner-freebsd-drivers@FreeBSD.ORG Mon Nov 24 17:39:02 2008 Return-Path: Delivered-To: freebsd-drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 88A651065672 for ; Mon, 24 Nov 2008 17:39:02 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 3902F8FC18 for ; Mon, 24 Nov 2008 17:39:02 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.2/8.14.1) with ESMTP id mAOHanLw038110; Mon, 24 Nov 2008 10:36:49 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Mon, 24 Nov 2008 10:38:16 -0700 (MST) Message-Id: <20081124.103816.1649771647.imp@bsdimp.com> To: huntting@glarp.com From: "M. Warner Losh" In-Reply-To: References: X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-drivers@freebsd.org Subject: Re: mutex quandry X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Nov 2008 17:39:02 -0000 In message: "Brad Huntting" writes: : Dear hackers: : : My device driver, like many, maintains a counter (sc_inuse) and a flag : (sc_running) to keep track of how many 'users' are using the device : instance variables (softc) and wheather the device is trying to : detach. I call the functions scm_reserver() and scm_leave() (see : below) before and after any block of code that uses the softc. Then, : in my detach() function I clear the sc_running flag and msleep() : waiting for the sc_inuse counter to drain to zero before continuing, : eventually destroying the very mutex I use to protect these two : variables. My problem is here: : : if (sc && mtx_initialized(&sc->sc_inuse_mtx)) { : mtx_lock(&sc->sc_inuse_mtx); : ... : : Unfortunately, { mtx_initialized(x) && (mtx_lock(x), 1); } is not : atomic. In the unlikely (but possible) event that the lock is : destroyed after mtx_initialized(x) and before mtx_lock(x), the system : will behave badly (probably panic). Why do you care that it is initialized? This mutex should be created in the attach routine, and nothing can race it there unless it is creating threads, registering interrupts, etc before initializing it. : It would seem to be a common problem for most device drivers to : implement this 'open/closing' lock where the lock starts out : zero-filled in the pre-open state and destroyed in the post-closed : state. So how should one deal with this? I could invent my own : locking with atomic_*() operations, but then I loose the advantages of : using standard system locks (like witness). Right. I guess I still don't understand why you need to check to see if it is initialized. You should never be destroying the mutex if there are other threads accessing it. I guess I'm having problems understanding why you are even doing the non-atomic test, and why it is needed. Once you remove the test, the code falls into standard norms. Warner : thanx in advance, : brad : : /* : * scm_reserve() increments a inuse and scm_release() keep track : * of how many customers are on site. To shut down we close the : * door (sc_running=0), but we dont actually turn out the lights : * untill the last customer has left (sc_inuse==0). : * : * If device is configured, this returns TRUE and caller must call : * scm_release() when done. If device is not configured returns : * FALSE and no further action is required. : */ : static int : scm_reserve(struct scmicro_softc *sc) : { : int r; : : if (sc && mtx_initialized(&sc->sc_inuse_mtx)) { : mtx_lock(&sc->sc_inuse_mtx); : KASSERT(sc->sc_inuse >= 0, "scmicro: scm_reserve() : sc_inuse <0!\n"); : r = ( sc->sc_running ? ++sc->sc_inuse : 0 ); : mtx_unlock(&sc->sc_inuse_mtx); : } else { : r = 0; : DPRINTF(("scmicro: scm_reserve() failed to reserve : sc=%p!\n", sc)); : } : : return (r); : } : : /* call this iff scm_reserve() returns true. */ : static inline void : scm_release(struct scmicro_softc *sc) : { : : mtx_lock(&sc->sc_inuse_mtx); : if (--sc->sc_inuse == 0) : wakeup(&sc->sc_inuse); : KASSERT(sc->sc_inuse >= 0, "scmicro: scm_release() sc_inuse <0!\n"); : mtx_unlock(&sc->sc_inuse_mtx); : } : : static int : scmicro_detach(device_t self) : { : /*....*/ : mtx_lock(&sc->sc_inuse_mtx); : sc->sc_running = 0; /* redundant */ : if (sc->sc_inuse > 0) : msleep_spin((void *)&sc->sc_inuse, &sc->sc_inuse_mtx, : "scmicro detach", 0); : mtx_unlock(&sc->sc_inuse_mtx); : /*....*/ : mtx_destroy(&sc->sc_inuse_mtx); : /*....*/ : } : _______________________________________________ : freebsd-drivers@freebsd.org mailing list : http://lists.freebsd.org/mailman/listinfo/freebsd-drivers : To unsubscribe, send any mail to "freebsd-drivers-unsubscribe@freebsd.org" : :