From owner-svn-src-all@FreeBSD.ORG Thu Jun 2 21:36:13 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 139F51065730; Thu, 2 Jun 2011 21:36:13 +0000 (UTC) (envelope-from ken@kdm.org) Received: from nargothrond.kdm.org (nargothrond.kdm.org [70.56.43.81]) by mx1.freebsd.org (Postfix) with ESMTP id C78858FC1A; Thu, 2 Jun 2011 21:36:12 +0000 (UTC) Received: from nargothrond.kdm.org (localhost [127.0.0.1]) by nargothrond.kdm.org (8.14.2/8.14.2) with ESMTP id p52LaBdY048885; Thu, 2 Jun 2011 15:36:11 -0600 (MDT) (envelope-from ken@nargothrond.kdm.org) Received: (from ken@localhost) by nargothrond.kdm.org (8.14.2/8.14.2/Submit) id p52LaBHu048884; Thu, 2 Jun 2011 15:36:11 -0600 (MDT) (envelope-from ken) Date: Thu, 2 Jun 2011 15:36:11 -0600 From: "Kenneth D. Merry" To: Bruce Evans Message-ID: <20110602213611.GA47880@nargothrond.kdm.org> References: <201105311729.p4VHTwrZ033296@svn.freebsd.org> <4DE5D72A.1020405@FreeBSD.org> <20110601193030.A1275@besplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110601193030.A1275@besplex.bde.org> User-Agent: Mutt/1.4.2i Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Andriy Gapon Subject: Re: svn commit: r222537 - in head/sys: kern sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jun 2011 21:36:13 -0000 On Wed, Jun 01, 2011 at 20:07:31 +1000, Bruce Evans wrote: > On Wed, 1 Jun 2011, Andriy Gapon wrote: > > >on 31/05/2011 20:29 Kenneth D. Merry said the following: > >>+ mtx_init(&mbp->msg_lock, "msgbuf", NULL, MTX_SPIN); > > > >Sorry that I didn't gather myself together for a review before this change > >got > >actually committed. > >Do you see any reason not to make this spinlock recursive? > > I see reasons why it must not exist. The message buffer code cannot use > any normal locking, and was carefully written to avoid doing so. > > >I am a little bit worried about "exotic" situations like receiving an NMI > >in the > >middle of printing and wanting to print in the NMI context, or similar > >things > >that penetrate contexts with disabled interrupts - e.g. Machine Check > >Exception. > >Also it's not clear to me if there won't any bigger damage in the > >situations > >like those described above. > > > >P.S. I have been thinking about fixing the problem in a different fashion, > >via > >reserving portions of dmesg buffer for a whole message using CAS: > >http://lists.freebsd.org/pipermail/freebsd-hackers/2010-April/031535.html > > Something like this might work. > > PRINTF_BUFR size should not exist either, especially now that it is > much more complicated and broken. Here is some of my old mail about > adding (necessarily not normal) locking to to printf. No one replied, > so I never finished this :-(. That particular solution doesn't lock the normal kernel printf path, and so won't fix what we're trying to fix. (We've got lots of disks in each system, and when there are various SES events and disks arriving and departing, there are multiple kernel contexts doing printfs simultaneously, and that results in scrambled dmesg output.) I think the goals should be: - console output, syslog output, and dmesg output are not scrambled. (i.e. individual kernel printfs make it out atomically, or at least with a certain granularity, like PRINTF_BUFR_SIZE.) - Can be called by any kernel routine (i.e. doesn't sleep) - Offers a string at a time interface. - Does the right thing for log messages (priority, etc.) It looks like we should add "does not use normal locking" to the list as well. As Justin mentioned, we started off down the path of using atomic operations, and then figured out that wouldn't fully work. Then we discussed doing a per-CPU message buffer, with each message tagged with a sequence number, and having the reader re-serialize the messages in the right order. The complexity started to get large enough that we decided that using a spin lock would be a much easier approach. cnputs() already uses a spinlock, so we're no worse off than before from a locking standpoint. We could perhaps make the message buffer mutex recursive and set the MTX_NOWITNESS flag as well, that might make things a little better from a side effect standpoint. If we can come up with a solution that meets the above goals, Justin and I would be happy to help implement it. Ken -- Kenneth Merry ken@FreeBSD.ORG