Re: [patch] mm: bug in set_page_dirty_buffers

[Nick Piggin <npiggin@suse.de>]

On Tue, Oct 10, 2006 at 01:07:42AM -0700, Andrew Morton wrote:
> On Tue, 10 Oct 2006 09:21:29 +0200
> Nick Piggin <npiggin@suse.de> wrote:
> 
> >  void block_invalidatepage(struct page *page, unsigned long offset)
> >  {
> > + 	struct address_space *mapping = page->mapping;
> >  	struct buffer_head *head, *bh, *next;
> > -	unsigned int curr_off = 0;
> > +	unsigned int curr_off;
> >  
> >  	BUG_ON(!PageLocked(page));
> >  	if (!page_has_buffers(page))
> >  		goto out;
> >  
> > +	curr_off = 0;
> >  	head = page_buffers(page);
> >  	bh = head;
> >  	do {
> > @@ -1455,6 +1457,24 @@ void block_invalidatepage(struct page *p
> >  		bh = next;
> >  	} while (bh != head);
> >  
> > +	/* strip the dirty bits and protect against concurrent set_page_dirty */
> > +	spin_lock(&mapping->private_lock);
> > +	curr_off = 0;
> > +	head = page_buffers(page);
> > +	bh = head;
> > +	do {
> > +		unsigned int next_off = curr_off + bh->b_size;
> > +		next = bh->b_this_page;
> > +
> > +		if (offset <= curr_off) {
> > +			clear_buffer_dirty(bh);
> > +			set_buffer_invalid(bh);
> > +		}
> > +		curr_off = next_off;
> > +		bh = next;
> > +	} while (bh != head);
> > +	spin_unlock(&mapping->private_lock);
> 
> If the buffer's redirtied after discard_buffer() got at it, we've got some
> nasty problems in there.
> 
> Are you sure this race can happen?  Nobody's allowed to have a page mapped
> while it's undergoing truncation (vmtruncate()).

Well, not technically. I think it can happen with nonlinear pages now,
but that is a bug in truncate and I have some patches to fix them.

But anyone who has done a get_user_pages, AFAIKS, can later run a
set_page_dirty on the pages. Nasty problem, but I think we are OK to
just ignore the dirty bits in this case, because the truncate might
well have happened _after_ the get_user_pages guy set the page dirty
anyway.

> There might be a problem with the final blocks in the page outside i_size. 
> iirc what happens here is that the bh outside i_size _is_ marked dirty, but
> writepage() will notice that it's outside i_size and will just mark it
> clean again without doing IO.

Didn't think of that. How does it get to writepage though, if it has
lost its mapping?

--
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>