Subject: Re: [jffs2] [rfc] fix write deadlock regression
From: David Woodhouse <dwmw2@infradead.org>
To: Nick Piggin <npiggin@suse.de>
Cc: Jason Lunz <lunz@falooley.org>, lkml <linux-kernel@vger.kernel.org>,
       jffs-dev@axis.com, Hugh Dickins <hugh@veritas.com>,
       Andrew Morton <akpm@linux-foundation.org>
In-Reply-To: <20070902042012.GA5864@wotan.suse.de>
References: <20070830182354.GA25077@falooley.org>
	 <20070831212636.GB12868@falooley.org> <20070901190602.GA5926@falooley.org>
	 <20070902042012.GA5864@wotan.suse.de>
Content-Type: text/plain
Date: Sun, 02 Sep 2007 13:13:23 +0100
Message-Id: <1188735203.3834.16.camel@shinybook.infradead.org>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1591
Lines: 37

Jason, thank you _so_ much for finding the underlying cause of this.

On Sun, 2007-09-02 at 06:20 +0200, Nick Piggin wrote:
> Hmm, thanks for that. It does sound like it is deadlocking via
> commit_write(). OTOH, it seems like it could be using the page
> before it is uptodate -- it _may_ only be dealing with uptodate
> data at that point... but if so, why even read_cache_page at
> all?

jffs2_readpage() is synchronous -- there's no chance that the page won't
be up to date. We're doing this for garbage collection -- if there are
many log entries covering a single page of data, we want to write out a
single replacement which covers the whole page, obsoleting the previous
suboptimal representation of the same data.

> However, it is a regression. So unless David can come up with a
> more satisfactory approach, I guess we'd have to go with your
> patch. 

I think Jason's patch is the best answer for the moment. At some point
in the very near future I want to improve the RAM usage and compression
ratio by dropping the rule that data nodes may not cross page boundaries
-- in which case garbage collection will need to do something other than
reading the page using read_cache_page() and then writing it out again;
it'll probably need to end up using its own internal buffer. But for
now, Jason's patch looks good.

Thanks.

-- 
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/