From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH] Fix ext2 error reporting on fsync Date: Wed, 18 Jan 2006 23:46:53 +0100 Message-ID: <20060118224652.GA6434@atrey.karlin.mff.cuni.cz> References: <20060111174302.GD16728@atrey.karlin.mff.cuni.cz> <20060112142656.GB14235@atrey.karlin.mff.cuni.cz> <20060112124717.6e242802.akpm@osdl.org> <200601122108.55103.mason@suse.com> <20060112181628.63c4bf39.akpm@osdl.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Chris Mason , linux-fsdevel@vger.kernel.org Return-path: Received: from atrey.karlin.mff.cuni.cz ([195.113.31.123]:32731 "EHLO atrey.karlin.mff.cuni.cz") by vger.kernel.org with ESMTP id S932561AbWARWqy (ORCPT ); Wed, 18 Jan 2006 17:46:54 -0500 To: Andrew Morton Content-Disposition: inline In-Reply-To: <20060112181628.63c4bf39.akpm@osdl.org> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org > Chris Mason wrote: > > > > On Thursday 12 January 2006 15:47, Andrew Morton wrote: > > > Jan Kara wrote: > > > > > Jan Kara wrote: > > > > > > > > > > hm, yes, try_to_free_buffers() against a blockdev page is a problem. > > > > > But I think it's fixable. > > > > > > > > > > In the second loop of drop_buffers(), if we find a bh which had a write > > > > > error we need to set AS_EIO on the address_space which is interested in > > > > > this buffer. > > > > > > > > > > That address_space is pinned by a) the fact that it has buffers at > > > > > ->private_list and b) we hold the blockdev mapping's private_lock. > > > > > > > > > > The only problem is actually _finding_ the address_space which is > > > > > interested in this buffer_head. Looks like we'd need a backpointer in > > > > > the buffer_head. > > > > > > > > Yes, the pointer seems to be inevitable in this solution. If you think > > > > the improvement is worth adding 4 bytes to each buffer_head, then I can > > > > write the patch. > > > > > > hm. It only affects what are now rarely-used filesystems like ext2, minix, > > > etc. Not sure about reiser3. But it is a strict correctness issue. I > > > suppose we should do it. It'd be nice to find a way to avoid increasing > > > the bh size.. > > > > > > > I know my patch is pretty nasty, but is it really worth adding complexity to > > the base code for this corner case? > > > > It's not much complexity: a few set_bits and test_and_clear_bits in three > places. The main drawback is a larger buffer_head. > > Yeah, it's a pita, but it is a data-integrity correctness thing. There's a way to avoid the extra pointer in buffer_head: we could change the circular linked list b_assoc_buffers to a non-circular one (probably use hlists). Then if we find a buffer with IO error, we could find a list head and from it compute the pointer to the mapping... It would not be fast but on IO error I think we could afford it. Do you think it's a good idea? Honza -- Jan Kara SuSE CR Labs