From owner-svn-src-projects@FreeBSD.ORG Mon Jul 30 21:00:23 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 5DA9C1065673; Mon, 30 Jul 2012 21:00:22 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com [209.85.217.182]) by mx1.freebsd.org (Postfix) with ESMTP id C2F8F8FC16; Mon, 30 Jul 2012 21:00:21 +0000 (UTC) Received: by lbon10 with SMTP id n10so4521393lbo.13 for ; Mon, 30 Jul 2012 14:00:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:reply-to:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=qBkxuc3+GxphM7mVHFHjVdLJkrUFhwgeqF364lMO4Ss=; b=FUwFXR6JgX/zYJwEO/1s+xCwJRfW1ZMVutK296cIBsfgitmzDQGTyOT/0mssDChJ6X zqxyGgzEIVR5XJcZweLYwz86W+BDhZ+VdSbWVNJjJiaRg/4nk2Mug1GwLTE6HvAE1Kly 4qgyBIQYRE10vhPIhSaLy+NyRA5g7kvZh4q8Rev8PWS1AUiGsW7UiOLg5oMKSgdkdeo6 /x4Lwu3DuisXnymZm3TGi7z0S5Qq/JVmfvZ72ATWFIjkGJpI+Uc555RGZ7QwtvstJzJj 6RfRIcz3wnAn/BIRBqhlcnlMoKfoQh4P1a6jqL6Zbv+OTA+K3rozGGMf/cuzR0gRLGz3 TRGA== MIME-Version: 1.0 Received: by 10.152.46.6 with SMTP id r6mr12801716lam.7.1343682020713; Mon, 30 Jul 2012 14:00:20 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.112.27.65 with HTTP; Mon, 30 Jul 2012 14:00:20 -0700 (PDT) In-Reply-To: References: <201207301350.q6UDobCI099069@svn.freebsd.org> <20120730143943.GY2676@deviant.kiev.zoral.com.ua> <201207301149.43458.jhb@freebsd.org> Date: Mon, 30 Jul 2012 22:00:20 +0100 X-Google-Sender-Auth: NYae1i_z3Hob1ZtRSIVCx5zLkz4 Message-ID: From: Attilio Rao To: John Baldwin Content-Type: text/plain; charset=UTF-8 Cc: Konstantin Belousov , Davide Italiano , src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: attilio@FreeBSD.org List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Jul 2012 21:00:23 -0000 On Mon, Jul 30, 2012 at 9:52 PM, Attilio Rao wrote: > On Mon, Jul 30, 2012 at 4:49 PM, John Baldwin wrote: >> On Monday, July 30, 2012 10:39:43 am Konstantin Belousov wrote: >>> On Mon, Jul 30, 2012 at 03:24:26PM +0100, Attilio Rao wrote: >>> > On 7/30/12, Davide Italiano wrote: >>> > > On Mon, Jul 30, 2012 at 4:02 PM, Attilio Rao >> wrote: >>> > > Thanks for the comment, Attilio. >>> > > Yes, it's exactly what you thought. If direct flag is equal to one >>> > > you're sure you're processing a callout which runs directly from >>> > > hardware interrupt context. In this case, the running thread cannot >>> > > sleep and it's likely you have TDP_NOSLEEPING flags set, failing the >>> > > KASSERT() in THREAD_NO_SLEEPING() and leading to panic if kernel is >>> > > compiled with INVARIANTS. >>> > > In case you're running from SWI context (direct equals to zero) code >>> > > remains the same as before. >>> > > I think what I'm doing works due the assumption thread running never >>> > > sleeps. Do you suggest some other way to handle this? >>> > >>> > Possibly the quicker way to do this is to have a way to deal with the >>> > TDP_NOSLEEPING flag in recursed way, thus implement the same logic as >>> > VFS_LOCK_GIANT() does, for example. >>> > You will need to change the few callers of THREAD_NO_SLEEPING(), but >>> > the patch should be no longer than 10/15 lines. >>> >>> There are already curthread_pflags_set/restore KPI designed exactly to >> handle >>> nested private thread flags. >>> >>> Also, I wonder, should you assert somehow that direct dispatch cannot block >>> as well ? >> >> Hmm, I have a nested TDP_NOSLEEPING already (need it to fix an issue in >> rmlocks). It uses a count though as the flag is set during rm_rlock() and >> released during rm_runlock(). I don't think it could use a set/restore KPI as >> there is no good place to store the state. > > Our stock rmlocks don't seem to use TDP_NOSLEEPING/THREAD_NO_SLEEPING > so I'm not entirely sure about the case you were trying to fix, can > you show the patch? I think I can see what you did. Did you add TDP_NOSLEEPING when acquiring the first rmlock and drop when the last was released. This is a nice property, in general exendible to all our blocking primitives, really. Right now some sort of similar check is enforced by WITNESS, but I can see a value in cases where you want to test a kernel with INVARIANTS but without WITNESS (it is not a matter of performance, it is just that sometimes you cannot reproduce some specific races with WITNESS on, while you can do it with WITNESS off, so it is funny to note how sometimes WITNESS should be just dropped for some locking issues). Attilio -- Peace can only be achieved by understanding - A. Einstein