From owner-freebsd-arch@FreeBSD.ORG Tue Aug 23 13:33:37 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 E5F1C1065670 for ; Tue, 23 Aug 2011 13:33:37 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 3ADA68FC18 for ; Tue, 23 Aug 2011 13:33:36 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id QAA27433; Tue, 23 Aug 2011 16:33:35 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4E53AC2F.1040006@FreeBSD.org> Date: Tue, 23 Aug 2011 16:33:35 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:5.0) Gecko/20110705 Thunderbird/5.0 MIME-Version: 1.0 To: mdf@FreeBSD.org References: <4E53986B.5000804@FreeBSD.org> In-Reply-To: X-Enigmail-Version: 1.2pre Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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:33:38 -0000 on 23/08/2011 15:58 mdf@FreeBSD.org said the following: > On Tue, Aug 23, 2011 at 5:09 AM, Andriy Gapon wrote: >> III. With my stop_scheduler_on_panic patch ukbd_poll() produces infinite chains >> of 'infinite' recursion because mtx_owned() always returns false. This is because >> I skip all lock/unlock/etc operations in the post-panic context. I think 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 this >> "lock-less" context, but _hypothetically_ there could exist some symmetric code >> that does something like: >> func() >> { >> if (mtx_owned(&Giant)) { >> mtx_unlock(&Giant); >> func(); >> mtx_lock(&Giant); >> return; >> } >> >> // 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. But, OTOH, there are snippets like this (found with grep, haven't looked at the code): /usr/src/sys/kern/kern_sx.c: while (mtx_owned(&Giant)) { >> That question III actually brings another thought: perhaps the whole of idea of >> skipping locks in a certain context is not a correct direction? >> Perhaps, instead we should identify all the code that could be legitimately >> executed in the after-panic and/or kdb contexts and make that could explicitly >> aware of its execution context. That is, instead of trying to make _lock, >> _unlock, _owned, _trylock, etc do the right thing auto-magically, we should try to >> make the calling code check panicstr and kdb_active and do the right thing 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. Yes, I agree with your and John's practical approach to this. Maybe print a warning about each skipped locking operation? But not sure if that would be useful, if there would be no intention of changing the code. -- Andriy Gapon