Re: [patch 6/9] mm: speculative get page

Hugh Dickins
2007-04-16T18:54:03Z

> --- linux-2.6.orig/include/linux/pagemap.h
> +++ linux-2.6/include/linux/pagemap.h
> ...
> +static inline int page_cache_get_speculative(struct page *page)
> +{
> + VM_BUG_ON(in_interrupt());
> +
> +#ifndef CONFIG_SMP
> +# ifdef CONFIG_PREEMPT
> + VM_BUG_ON(!in_atomic());
> +# endif
> + /*
> + * Preempt must be disabled here - we rely on rcu_read_lock doing
> + * this for us.
> + *
> + * Pagecache won't be truncated from interrupt context, so if we have
> + * found a page in the radix tree here, we have pinned its refcount by
> + * disabling preempt, and hence no need for the "speculative get" that
> + * SMP requires.
> + */
> + VM_BUG_ON(page_count(page) == 0);
> + atomic_inc(&page->_count);
> +
> +#else
> + if (unlikely(!get_page_unless_zero(page)))
> + return 0; /* page has been freed */

Now you're using get_page_unless_zero() here, you need to remove its
VM_BUG_ON(PageCompound(page));
since hugetlb_nopage() uses find_lock_page() on huge compound pages
and so comes here (and you have a superior VM_BUG_ON further down).

You could move that VM_BUG_ON to its original caller isolate_lru_pages(),
or you could replace it by your superior check in get_page_unless_zero();
but I'd be inclined to do the easiest and just cut it out now.

Hugh