From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Mahoney Subject: Re: [PATCH] ext3: ext3_commit_super should always mark super uptodate Date: Fri, 19 Dec 2008 10:46:42 -0500 Message-ID: <494BC1E2.1010901@suse.com> References: <4947EFCE.8020605@suse.com> <494BC0B9.2050007@cfl.rr.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: linux-fsdevel@vger.kernel.org To: Phillip Susi Return-path: Received: from cantor2.suse.de ([195.135.220.15]:60712 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751005AbYLSPqs (ORCPT ); Fri, 19 Dec 2008 10:46:48 -0500 In-Reply-To: <494BC0B9.2050007@cfl.rr.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Phillip Susi wrote: > Jeff Mahoney wrote: >> After a superblock write failure, the buffer_head is marked !uptodate. >> Since the superblock is something of an exception -- read once and >> a reference kept for the duration of the mount -- it is by definition >> always uptodate. > > Not if it has been modified and not yet written back to disk. Then it > is dirty by definition. I think you're missing the distinction between uptodate and dirty here. >> This is somewhat academic for the most part until we encounter error >> conditions. For example, if a disk goes away in a SAN environment, the >> write failure will occur and it will be followed by others. ext3_error >> wants to mark the superblock dirty via ext3_commit_super, but once >> the first write fails, the subsequent mark_buffer_dirty calls will >> issue warnings because the buffer is not uptodate. > > Why on earth is a warning issued for marking an already dirty buffer > dirty again? Surly buffers can be modified again and thus, marked as > dirty a second time before pdflush gets around to syncing them all the > time. No, it issues a warning when you mark a !uptodate buffer dirty. The idea behind it is that the buffer contents may be stale with respect to disk contents. In the case of the superblock, the in-memory contents are always the most recent and it *never* re-reads it from disk. >> + set_buffer_uptodate(sbh); >> mark_buffer_dirty(sbh); >> if (sync) >> sync_dirty_buffer(sbh); > > Are you sure this only matters when there is an IO error? It looks > there like if !sync then the sb can continue to sit in memory marked as > dirty for some time before someone tries to flush it to disk. Eventually > can't we end up back through this code patch and calling > mark_buffer_dirty again before it was ever flushed in the first place, > even without an IO error? Again, uptodate != dirty. - -Jeff - -- Jeff Mahoney SUSE Labs -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAklLweIACgkQLPWxlyuTD7KbKACfTn4daFFerN0X/V5rpt+eSK6C p7MAnjDOIZAF2EUb6gWqWk+Mw4/6ECPl =GPkb -----END PGP SIGNATURE-----