From owner-svn-src-head@FreeBSD.ORG Thu May 21 15:43:58 2009 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E2886106566C; Thu, 21 May 2009 15:43:58 +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 802018FC2D; Thu, 21 May 2009 15:43:58 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id n4LFenVT077840; Thu, 21 May 2009 09:40:50 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Thu, 21 May 2009 09:41:00 -0600 (MDT) Message-Id: <20090521.094100.70797067.imp@bsdimp.com> To: rwatson@FreeBSD.org From: "M. Warner Losh" In-Reply-To: References: <3bbf2fe10905210629p46c7a204v6863aaba77354462@mail.gmail.com> <200905210942.35555.jhb@freebsd.org> 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: src-committers@FreeBSD.org, jhb@FreeBSD.org, svn-src-all@FreeBSD.org, attilio@FreeBSD.org, svn-src-head@FreeBSD.org, kostikbel@gmail.com Subject: Re: svn commit: r192535 - head/sys/kern X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 May 2009 15:43:59 -0000 In message: Robert Watson writes: : On Thu, 21 May 2009, John Baldwin wrote: : : >>>> Move the M_WAITOK flag in notify() into an M_NOWAIT one in order to : > match : >>>> the behaviour alredy present with the further malloc() call in : >>>> devctl_notify(). : >>>> This fixes a bug in the CAM layer where the camisr handler finished to : >>>> call camperiphfree() (and subsequently destroy_dev() resulting in a new : >>>> dev notify) while the xpt lock is held. : >>> This is wrong. You cannot call destroy_dev() while holding any mutex. : >>> Taking this into account, it makes no sense to use M_NOWAIT in notify(). : >> : >> As long as devctl_notify() also calls M_NOWAIT and if not available skips : >> "silently" it just does the same thing, I think this approach is more : >> consistent. : >> : >> It remains, though, the fact to fix CAM when calling destroy_dev(). Maybe : >> we should add a witness_warn() there? : > : > I agree with kib, this should be reverted and CAM fixed instead. I also : > agree that M_NOWAIT use should be limited where possible. : : devctl_notify() probably needs to grow a sleepable flag, or perhaps we need : two variations, one that can sleep. devctl_notify() has expanded well beyond its original needs. Having an extra case for sleeping is the wrong way to solve this problem. Really. We're adding hacks on hacks on hacks here and we need to step back and think. I specifically didn't put in CDEV notifications into devd when I originally did it because one can get the same notification via kevents on /dev. Maybe the right answer is to remove this stuff entirely and update devd to do that instead? It isn't a lot of code, and should provide equivalent functionality without needing to change the rules of the game when it comes to destroy_dev(). Especially this close to the code slush... Comments? Warner