From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH 004 of 4] Make address_space_operations->invalidatepage
 return void
Date: Mon, 13 Mar 2006 13:36:25 -0800
Message-ID: <20060313133625.26496547.akpm@osdl.org>
References: <20060313104910.15881.patches@notabene>
	<1060312235331.15985@suse.de>
	<1142267531.9971.5.camel@kleikamp.austin.ibm.com>
	<1142277225.9949.3.camel@kleikamp.austin.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: neilb@suse.de, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from smtp.osdl.org ([65.172.181.4]:29319 "EHLO smtp.osdl.org")
	by vger.kernel.org with ESMTP id S932461AbWCMViy (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Mon, 13 Mar 2006 16:38:54 -0500
To: Dave Kleikamp <shaggy@austin.ibm.com>
In-Reply-To: <1142277225.9949.3.camel@kleikamp.austin.ibm.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-Id: linux-fsdevel.vger.kernel.org

Dave Kleikamp <shaggy@austin.ibm.com> wrote:
>
> On Mon, 2006-03-13 at 10:32 -0600, Dave Kleikamp wrote:
>  > I'll try to stress test jfs with these patches to see if I can trigger
>  > the an oops here.
> 
>  While stress testing on a jfs volume (dbench), I hit an assert in jbd:
> 
>  Assertion failure in journal_invalidatepage() at fs/jbd/transaction.c:1920: "!page_has_buffers(page)"

Yes, thanks, that assertion has become wrong.

--- devel/fs/jbd/transaction.c~make-address_space_operations-invalidatepage-return-void-jbd-fix	2006-03-13 13:33:12.000000000 -0800
+++ devel-akpm/fs/jbd/transaction.c	2006-03-13 13:33:12.000000000 -0800
@@ -1915,9 +1915,8 @@ void journal_invalidatepage(journal_t *j
 	} while (bh != head);
 
 	if (!offset) {
-		/* Maybe should BUG_ON !may_free - neilb */
-		try_to_free_buffers(page);
-		J_ASSERT(!page_has_buffers(page));
+		if (may_free && try_to_free_buffers(page))
+			J_ASSERT(!page_has_buffers(page));
 	}
 }
 

However I'm more inclined to drop the whole patch, really - having
->invalidatepage() return a success indication makes sense.  The fact that
we're currently not using that return value doesn't mean that we shouldn't,
didn't and won't.