Re: [patch] mm: bug in set_page_dirty_buffers

[Andrew Morton <akpm@osdl.org>]

On Tue, 10 Oct 2006 07:48:33 +0200
Nick Piggin <npiggin@suse.de> wrote:

> On Mon, Oct 09, 2006 at 10:29:05PM -0700, Andrew Morton wrote:
> > On Tue, 10 Oct 2006 07:22:48 +0200
> > Nick Piggin <npiggin@suse.de> wrote:
> > 
> > > > > AFAIKS, it is just fs/buffer.c that is racy.
> > > > 
> > > > Need to review all ->set_page_dirty, ->writepage, ->invalidatepage, ->etc
> > > > implementations before we can say that.
> > 
> >     ^^^ this
> 
> ->writepage is called under page lock.
> 
> ->invalidatepage is called under page lock.
> 
> I think ->spd is the only one to worry about.
> 
> __set_page_dirty_nobuffers does use the tree_lock to ensure it hasn't been
> truncated. However it doe leave orphaned clean buffers which vmscan cannot
> reclaim. So probably we should not dirty it *until* we have verified it
> is still part of a mapping.
> 
> Similarly for __set_page_dirty_buffers.
> 
> Comments in ext3 look like it has spd under control.
> Reiserfs looks similar, but it does have the unchecked
> page->mapping problem that I fixed for spd_buffers.
> 
> Am I missing something?

Well it's a matter of reviewing all codepaths in the kernel which
manipulate internal page state and see if they're racy against their
->set_page_dirty().  All because of zap_pte_range().

The page lock protects internal page state.  It'd be better to fix
zap_pte_range().

How about we trylock the page and if that fails, back out and drop locks
and lock the page then dirty it and then resume the zap?  Negligible
overhead, would be nice and simple apart from that i_mmap_lock thing.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>