From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mingming Cao Subject: Re: [patch][rfc] fs: fix nobh error handling Date: Wed, 08 Aug 2007 07:39:42 -0700 Message-ID: <1186583982.20310.4.camel@localhost.localdomain> References: <20070807055129.GE17986@wotan.suse.de> <20070807180903.3cf36b77.akpm@linux-foundation.org> <20070808021838.GA11018@wotan.suse.de> <20070807193347.fbcd7f38.akpm@linux-foundation.org> <20070808031235.GD11018@wotan.suse.de> <1186578456.6657.11.camel@norville.austin.ibm.com> Reply-To: cmm@us.ibm.com Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Nick Piggin , Andrew Morton , linux-fsdevel@vger.kernel.org, Badari Pulavarty To: Dave Kleikamp Return-path: Received: from e1.ny.us.ibm.com ([32.97.182.141]:33616 "EHLO e1.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935478AbXHHOkv (ORCPT ); Wed, 8 Aug 2007 10:40:51 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e1.ny.us.ibm.com (8.13.8/8.13.8) with ESMTP id l78EdikR027601 for ; Wed, 8 Aug 2007 10:39:44 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v8.4) with ESMTP id l78Edi7s530386 for ; Wed, 8 Aug 2007 10:39:44 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id l78EdhdN020496 for ; Wed, 8 Aug 2007 10:39:44 -0400 In-Reply-To: <1186578456.6657.11.camel@norville.austin.ibm.com> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Wed, 2007-08-08 at 08:07 -0500, Dave Kleikamp wrote: > On Wed, 2007-08-08 at 05:12 +0200, Nick Piggin wrote: > > On Tue, Aug 07, 2007 at 07:33:47PM -0700, Andrew Morton wrote: > > > On Wed, 8 Aug 2007 04:18:38 +0200 Nick Piggin wrote: > > > > > > > On Tue, Aug 07, 2007 at 06:09:03PM -0700, Andrew Morton wrote: > > > > > > > > > > With this change, nobh_prepare_write() can magically attach buffers to the > > > > > page. But a filesystem which is running in nobh mode wouldn't expect that, > > > > > and could quite legitimately go BUG, or leak data or, more seriously and > > > > > much less fixably, just go and overwrite page->private, because it "knows" > > > > > that nobody else is using ->private. > > > > > > > > I was fairly sure that a filesystem can not assume buffers won't be > > > > attached, because there are various error case paths thta do exactly > > > > the same thing (eg. nobh_writepage can call __block_write_full_page > > > > which will attach buffers). > > > > > > oh crap, that's sad. Either we broke it later on or I misremembered. > > > > > > > Does any filesystem assume this? Is it not already broken? > > > > > > Yes, it would be broken. > > > > > > > > > > > > I'd have thought that it would be better to not attach the buffers and to > > > > > go ahead and do whatever synchronous IO is needed in the error recovery > > > > > code, then free those buffers again. > > > > > > > > It is hard because if the synchronous IO fails, then what do you do? > > > > > > Do what we usually do when an IO error happens: crash the kernel? (Sorry, > > > have been spending too long at bugzilla.kernel.org) > > > > Heh.. I guess there is still a chance to retry the IO with sync or > > fsync. I'mt not surprised if the "normal" pagecache error handling > > paths doesn't work so well either, but at least if we can duplicate > > as little code as possible it might get fixed up one day. > > > > > > > > > > You could try making it up as you go along, but of course if we _can_ > > > > attach the buffers here then it would be preferable to do that. IMO. > > > > > > > > > > > > > Also, you have a couple of (cheerily uncommented) PagePrivate() tests in > > > > > there which should be page_has_buffers(). > > > > > > > > Yeah, I guess the whole thing needs more commenting :P > > > > page_has_buffers... right, I'll change that. > > > > > > Did it get much testing? > > > > A little. Obviously it only really changes anything when an IO error hits, > > and I found that ext3/jbd gives up and goes readonly pretty quickly when I > > inject IO errors into the block device. What I really want to do is just > > inject faults at nobh_prepare_write and do some longer tests. > > > > I'll do that today. > > For jfs's sake, I don't really care if it ever uses nobh again. I > originally started using it because I figured the movement was away from > buffer heads and jfs seemed just as happy with the nobh functions (after > a few bugs were flushed out). I don't think jfs really benefitted > though. > > That said, I don't really know who cares about the nobh option in ext3. > Actually IBM/LTC use the nobh option in ext3 on our internal kernel development server, to control the consumption of large amount of low memory space. Mingming