Re: truncate_list_pages() BUG and confusion

[Andrew Morton <akpm@zip.com.au>]

"Martin J. Bligh" wrote:
> 
> ..
> > If the page_cache_release() in truncate_complete_page() is calling
> > __free_pages_ok() then something really horrid has happened.
> 
> That's exactly what's happening.
> 
> > Yes, it could be that the page has had its refcount incorrectly
> > decremented somewhere.
> 
> I don't see you need that to make this bug happen.
> Say count is 0 when we enter truncate_list_pages.

It mustn't be zero!  The page is *known* to be in the pagecache,
so its count must be at least 1.

> We increment it.
> It's now 1 when we call page_cache_release.
> put_page_testzero dec's it back to 0, and returns true.
> We do a __free_pages_ok. Page is still locked. BUG.
> 
> No other process, nothing funky happening, no races, no other
> refcount decrements. Or that's the way I read it.
> 
> > Or the page wasn't in the pagecache at all.
> 
> The only thing I can think of was the pagecount shouldn't have been 0
> to start with (or the code path we're reading is wrong ;-) )

yup.  You can stick an

	if (page_count(page) == 0)
		BUG();

at the top of truncate_list_pages...

Actually it might help to add some extra checks to
page_cache_release():

1: If the page is in the pagecache, that's one.
2: If the page has buffers, that's another.

so 

  void page_cache_release(struct page *page)  
  {
+	int expected = 0;
+
+	if (page->buffers)
+		expected++;
+	if (page->mapping) {
+		if (!list_empty(page->list))
+			BUG();
+		expected++;
+	}
+
+	expected++;	/* The caller has a ref too */
+
+	if (page_count(page) < expected)
+		BUG();
+
        if (!PageReserved(page) && put_page_testzero(page)) {
                if (PageLRU(page))
                        lru_cache_del(page);
                __free_pages_ok(page, 0);
        }
  }

The list_empty() check will require that remove_page_from_inode_queue()
use list_del_init() instead of list_del().

The above code (untested, and racy as hell on SMP and preempt) may
catch whoever is droppng the refcount at the wrong time.

-