Re: [RFC][PATCH] page->mapping clarification [1/3] base functions

[Christoph Lameter <clameter@sgi.com>]

On Wed, 19 Sep 2007, KAMEZAWA Hiroyuki wrote:

> Any comments are welcome.

I am still a bit confused as to what the benefit of this is.

> Following functions are added
> 
>  * page_mapping_cache() ... returns address space if a page is page cache
>  * page_mapping_anon()  ... returns anon_vma if a page is anonymous page.
>  * page_is_pagecache()  ... returns true if a page is page-cache.
>  * page_inode()         ... returns inode which a page-cache belongs to.
>  * is_page_consistent() ... returns true if a page is still valid page cache 

Ok this could make the code more readable.

> +/*
> + * On an anonymous page mapped into a user virtual memory area,
> + * page->mapping points to its anon_vma, not to a struct address_space;
> + * with the PAGE_MAPPING_ANON bit set to distinguish it.
> + *
> + * Please note that, confusingly, "page_mapping" refers to the inode
> + * address_space which maps the page from disk; whereas "page_mapped"
> + * refers to user virtual address space into which the page is mapped.
> + */
> +#define PAGE_MAPPING_ANON       1
> +
> +static inline bool PageAnon(struct page *page)

bool??? That is unusual?

> +static inline struct address_space *page_mapping_cache(struct page *page)
> +{
> +	if (!page->mapping || PageAnon(page))
> +		return NULL;
> +	return page->mapping;
> +}

That is confusing.

if (PageAnon(page))
	return NULL;
return page->mapping;


> +static inline struct address_space *page_mapping(struct page *page)
> +{
> +	struct address_space *mapping = page->mapping;
> +
> +	VM_BUG_ON(PageSlab(page));
> +	if (unlikely(PageSwapCache(page)))
> +		mapping = &swapper_space;
> +#ifdef CONFIG_SLUB
> +	else if (unlikely(PageSlab(page)))
> +		mapping = NULL;
> +#endif

The #ifdef does not exist in rc6-mm1. No need to reintroduce it.

> +static inline bool
> +is_page_consistent(struct page *page, struct address_space *mapping)
> +{
> +	struct address_space *check = page_mapping_cache(page);
> +	return (check == mapping);
> +}

Why do we need a special function? Why is it safer?

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