From owner-freebsd-arch@FreeBSD.ORG Tue Aug 23 13:29:29 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BAF69106567B; Tue, 23 Aug 2011 13:29:29 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-qw0-f54.google.com (mail-qw0-f54.google.com [209.85.216.54]) by mx1.freebsd.org (Postfix) with ESMTP id 5D9B68FC0A; Tue, 23 Aug 2011 13:29:29 +0000 (UTC) Received: by qwc9 with SMTP id 9so80594qwc.13 for ; Tue, 23 Aug 2011 06:29:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=vMuZpJEjSZQCPuDfqwYLL9gKywbh9NQmhgyvWGh6ojQ=; b=YHiyW4aIN04pSQyGJV5988Rzf4AlpVgcZnHMSs7SJp1sol31IDat6zvGLmbUO04Hwt VsUKvFdZCwM1+MDGAGBfL+TXVLs06q5c388FGRMTjfIobIjIZ9LfnbcxbTrmzqu+I3aY /Hwfyb6gV2MoKcOMlMQ4CcA5j02tR/f2XDLvw= MIME-Version: 1.0 Received: by 10.229.67.65 with SMTP id q1mr2200741qci.246.1314104294902; Tue, 23 Aug 2011 05:58:14 -0700 (PDT) Sender: mdf356@gmail.com Received: by 10.229.98.8 with HTTP; Tue, 23 Aug 2011 05:58:14 -0700 (PDT) In-Reply-To: <4E53986B.5000804@FreeBSD.org> References: <4E53986B.5000804@FreeBSD.org> Date: Tue, 23 Aug 2011 05:58:14 -0700 X-Google-Sender-Auth: LA_e7_ZE1XEY2GweeExiwGhG3KQ Message-ID: From: mdf@FreeBSD.org To: Andriy Gapon Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-arch@freebsd.org Subject: Re: skipping locks, mutex_owned, usb X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Aug 2011 13:29:29 -0000 On Tue, Aug 23, 2011 at 5:09 AM, Andriy Gapon wrote: > III. =A0With my stop_scheduler_on_panic patch ukbd_poll() produces infini= te chains > of 'infinite' recursion because mtx_owned() always returns false. =A0This= is because > I skip all lock/unlock/etc operations in the post-panic context. =A0I thi= nk that > it's a good philosophical question: what operations like mtx_owned(), > mtx_recursed(), mtx_trylock() 'should' return when we actually act as if = no locks > exist at all? > > My first knee-jerk reaction was to change mtx_owned() to return true in t= his > "lock-less" context, but _hypothetically_ there could exist some symmetri= c code > that does something like: > func() > { > =A0 =A0 =A0 =A0if (mtx_owned(&Giant)) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mtx_unlock(&Giant); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0func(); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mtx_lock(&Giant); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return; > =A0 =A0 =A0 =A0} > > =A0 =A0 =A0 =A0// etc ... > > What do you think about this problem? Given that checking for a lock being held is a lot more common than checking if it's not held (in the context of mtx_assert(9) and friends), it seems reasonable to report that a lock is held in the special context of after panic. > That question III actually brings another thought: perhaps the whole of i= dea of > skipping locks in a certain context is not a correct direction? > Perhaps, instead we should identify all the code that could be legitimate= ly > executed in the after-panic and/or kdb contexts and make that could expli= citly > aware of its execution context. =A0That is, instead of trying to make _lo= ck, > _unlock, _owned, _trylock, etc do the right thing auto-magically, we shou= ld try to > make the calling code check panicstr and kdb_active and do the right thin= g on that > level (which would include not invoking those lock-related operations or = other > inappropriate operations). I don't think it's possible to identify all the code, since what runs after panic isn't tested very much. :-) I don't disagree that one should think about the issue, but providing reasonable defaults like skipping locks reduces the burden on the programmer. Thanks, matthew