* [RFC][PATCH] page->mapping clarification [2/3] changes in /mm
2007-09-19 7:43 [RFC][PATCH] page->mapping clarification [1/3] base functions KAMEZAWA Hiroyuki
@ 2007-09-19 7:44 ` KAMEZAWA Hiroyuki
2007-09-19 7:46 ` [RFC][PATCH] page->mapping clarification [3/3] changes in /fs generic KAMEZAWA Hiroyuki
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: KAMEZAWA Hiroyuki @ 2007-09-19 7:44 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: LKML, linux-mm, Andrew Morton, nickpiggin, ricknu-0
Make use of page-cache.h functions in /mm layer.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
mm/filemap.c | 19 ++++++++++---------
mm/memory.c | 5 +++--
mm/migrate.c | 8 ++------
mm/page-writeback.c | 4 ++--
mm/rmap.c | 36 ++++++++++++++----------------------
mm/shmem.c | 4 ++--
mm/truncate.c | 15 ++++++++-------
7 files changed, 41 insertions(+), 50 deletions(-)
Index: linux-2.6.23-rc6-mm1/mm/filemap.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/mm/filemap.c
+++ linux-2.6.23-rc6-mm1/mm/filemap.c
@@ -115,7 +115,7 @@ generic_file_direct_IO(int rw, struct ki
*/
void __remove_from_page_cache(struct page *page)
{
- struct address_space *mapping = page->mapping;
+ struct address_space *mapping = page_mapping_cache(page);
mem_container_uncharge_page(page);
radix_tree_delete(&mapping->page_tree, page->index);
@@ -127,7 +127,7 @@ void __remove_from_page_cache(struct pag
void remove_from_page_cache(struct page *page)
{
- struct address_space *mapping = page->mapping;
+ struct address_space *mapping = page_mapping_cache(page);
BUG_ON(!PageLocked(page));
@@ -641,7 +641,7 @@ repeat:
__lock_page(page);
/* Has the page been truncated while we slept? */
- if (unlikely(page->mapping != mapping)) {
+ if (unlikely(!is_page_consistent(page, mapping))) {
unlock_page(page);
page_cache_release(page);
goto repeat;
@@ -750,7 +750,7 @@ unsigned find_get_pages_contig(struct ad
ret = radix_tree_gang_lookup(&mapping->page_tree,
(void **)pages, index, nr_pages);
for (i = 0; i < ret; i++) {
- if (pages[i]->mapping == NULL || pages[i]->index != index)
+ if (!page_is_pagecache(pages[i]) || pages[i]->index != index)
break;
page_cache_get(pages[i]);
@@ -979,7 +979,7 @@ page_not_up_to_date:
lock_page(page);
/* Did it get truncated before we got the lock? */
- if (!page->mapping) {
+ if (!page_is_pagecache(page)) {
unlock_page(page);
page_cache_release(page);
continue;
@@ -1006,7 +1006,7 @@ readpage:
if (!PageUptodate(page)) {
lock_page(page);
if (!PageUptodate(page)) {
- if (page->mapping == NULL) {
+ if (!page_is_pagecache(page)) {
/*
* invalidate_inode_pages got it
*/
@@ -1545,7 +1545,7 @@ retry:
goto out;
lock_page(page);
- if (!page->mapping) {
+ if (!page_is_pagecache(page)) {
unlock_page(page);
page_cache_release(page);
goto retry;
@@ -2112,7 +2112,8 @@ static ssize_t generic_perform_write_2co
* use a non-zeroing copy, but the APIs aren't too
* consistent.
*/
- if (unlikely(!page->mapping || PageUptodate(page))) {
+ if (unlikely(!page_is_pagecache(page) ||
+ PageUptodate(page))) {
unlock_page(page);
page_cache_release(page);
page_cache_release(src_page);
@@ -2555,7 +2556,7 @@ out:
*/
int try_to_release_page(struct page *page, gfp_t gfp_mask)
{
- struct address_space * const mapping = page->mapping;
+ struct address_space * const mapping = page_mapping_cache(page);
BUG_ON(!PageLocked(page));
if (PageWriteback(page))
Index: linux-2.6.23-rc6-mm1/mm/memory.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/mm/memory.c
+++ linux-2.6.23-rc6-mm1/mm/memory.c
@@ -650,7 +650,8 @@ static unsigned long zap_pte_range(struc
* unmap shared but keep private pages.
*/
if (details->check_mapping &&
- details->check_mapping != page->mapping)
+ !is_page_consistent(page,
+ details->check_mapping))
continue;
/*
* Each page->index must be checked when
@@ -2310,7 +2311,7 @@ static int __do_fault(struct mm_struct *
* reworking page_mkwrite locking API, which
* is better done later.
*/
- if (!page->mapping) {
+ if (!page_is_pagecache(page)) {
ret = 0;
anon = 1; /* no anon but release vmf.page */
goto out;
Index: linux-2.6.23-rc6-mm1/mm/migrate.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/mm/migrate.c
+++ linux-2.6.23-rc6-mm1/mm/migrate.c
@@ -223,17 +223,13 @@ static void remove_anon_migration_ptes(s
{
struct anon_vma *anon_vma;
struct vm_area_struct *vma;
- unsigned long mapping;
-
- mapping = (unsigned long)new->mapping;
- if (!mapping || (mapping & PAGE_MAPPING_ANON) == 0)
+ anon_vma = page_mapping_anon(new);
+ if (!anon_vma)
return;
-
/*
* We hold the mmap_sem lock. So no need to call page_lock_anon_vma.
*/
- anon_vma = (struct anon_vma *) (mapping - PAGE_MAPPING_ANON);
spin_lock(&anon_vma->lock);
list_for_each_entry(vma, &anon_vma->head, anon_vma_node)
Index: linux-2.6.23-rc6-mm1/mm/page-writeback.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/mm/page-writeback.c
+++ linux-2.6.23-rc6-mm1/mm/page-writeback.c
@@ -832,7 +832,7 @@ retry:
*/
lock_page(page);
- if (unlikely(page->mapping != mapping)) {
+ if (unlikely(!is_page_consistent(page, mapping))) {
unlock_page(page);
continue;
}
@@ -940,7 +940,7 @@ int do_writepages(struct address_space *
*/
int write_one_page(struct page *page, int wait)
{
- struct address_space *mapping = page->mapping;
+ struct address_space *mapping = page_mapping_cache(page);
int ret = 0;
struct writeback_control wbc = {
.sync_mode = WB_SYNC_ALL,
Index: linux-2.6.23-rc6-mm1/mm/rmap.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/mm/rmap.c
+++ linux-2.6.23-rc6-mm1/mm/rmap.c
@@ -159,16 +159,13 @@ void __init anon_vma_init(void)
static struct anon_vma *page_lock_anon_vma(struct page *page)
{
struct anon_vma *anon_vma;
- unsigned long anon_mapping;
rcu_read_lock();
- anon_mapping = (unsigned long) page->mapping;
- if (!(anon_mapping & PAGE_MAPPING_ANON))
+ anon_vma = page_mapping_anon(page);
+ if (!anon_vma)
goto out;
if (!page_mapped(page))
goto out;
-
- anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON);
spin_lock(&anon_vma->lock);
return anon_vma;
out:
@@ -207,12 +204,11 @@ vma_address(struct page *page, struct vm
unsigned long page_address_in_vma(struct page *page, struct vm_area_struct *vma)
{
if (PageAnon(page)) {
- if ((void *)vma->anon_vma !=
- (void *)page->mapping - PAGE_MAPPING_ANON)
+ if (vma->anon_vma != page_mapping_anon(page))
return -EFAULT;
- } else if (page->mapping && !(vma->vm_flags & VM_NONLINEAR)) {
+ } else if (page_is_pagecache(page) && !(vma->vm_flags & VM_NONLINEAR)) {
if (!vma->vm_file ||
- vma->vm_file->f_mapping != page->mapping)
+ !is_page_consistent(page, vma->vm_file->f_mapping))
return -EFAULT;
} else
return -EFAULT;
@@ -333,9 +329,9 @@ static int page_referenced_anon(struct p
* @page: the page we're checking references on.
*
* For an object-based mapped page, find all the places it is mapped and
- * check/clear the referenced flag. This is done by following the page->mapping
- * pointer, then walking the chain of vmas it holds. It returns the number
- * of references it found.
+ * check/clear the referenced flag. This is done by following the
+ * address_space of page_maping_cache(), then walking the chain of vmas it
+ * holds. It returns the number of references it found.
*
* This function is only called from page_referenced for object-based pages.
*/
@@ -343,18 +339,16 @@ static int page_referenced_file(struct p
struct mem_container *mem_cont)
{
unsigned int mapcount;
- struct address_space *mapping = page->mapping;
+ struct address_space *mapping = page_mapping_cache(page);
pgoff_t pgoff = page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
struct vm_area_struct *vma;
struct prio_tree_iter iter;
int referenced = 0;
/*
- * The caller's checks on page->mapping and !PageAnon have made
- * sure that this is a file page: the check for page->mapping
- * excludes the case just before it gets set on an anon page.
+ * Make sure this is a file page.
*/
- BUG_ON(PageAnon(page));
+ BUG_ON(!mapping);
/*
* The page lock not only makes sure that page->mapping cannot
@@ -421,7 +415,7 @@ int page_referenced(struct page *page, i
else if (TestSetPageLocked(page))
referenced++;
else {
- if (page->mapping)
+ if (page_is_pagecache(page))
referenced +=
page_referenced_file(page, mem_cont);
unlock_page(page);
@@ -546,9 +540,7 @@ static void __page_check_anon_rmap(struc
* are initially only visible via the pagetables, and the pte is locked
* over the call to page_add_new_anon_rmap.
*/
- struct anon_vma *anon_vma = vma->anon_vma;
- anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
- BUG_ON(page->mapping != (struct address_space *)anon_vma);
+ BUG_ON(vma->anon_vma != page_mapping_anon(page));
BUG_ON(page->index != linear_page_index(vma, address));
#endif
}
@@ -893,7 +885,7 @@ static int try_to_unmap_anon(struct page
*/
static int try_to_unmap_file(struct page *page, int migration)
{
- struct address_space *mapping = page->mapping;
+ struct address_space *mapping = page_mapping_cache(page);
pgoff_t pgoff = page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
struct vm_area_struct *vma;
struct prio_tree_iter iter;
Index: linux-2.6.23-rc6-mm1/mm/shmem.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/mm/shmem.c
+++ linux-2.6.23-rc6-mm1/mm/shmem.c
@@ -917,7 +917,7 @@ static int shmem_writepage(struct page *
BUG_ON(!PageLocked(page));
BUG_ON(page_mapped(page));
- mapping = page->mapping;
+ mapping = page_mapping_cache(page);
index = page->index;
inode = mapping->host;
info = SHMEM_I(inode);
@@ -1454,7 +1454,7 @@ static const struct inode_operations shm
*/
static int shmem_readpage(struct file *file, struct page *page)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
int error = shmem_getpage(inode, page->index, &page, SGP_CACHE, NULL);
unlock_page(page);
return error;
Index: linux-2.6.23-rc6-mm1/mm/truncate.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/mm/truncate.c
+++ linux-2.6.23-rc6-mm1/mm/truncate.c
@@ -37,7 +37,7 @@
void do_invalidatepage(struct page *page, unsigned long offset)
{
void (*invalidatepage)(struct page *, unsigned long);
- invalidatepage = page->mapping->a_ops->invalidatepage;
+ invalidatepage = page_mapping_cache(page)->a_ops->invalidatepage;
#ifdef CONFIG_BLOCK
if (!invalidatepage)
invalidatepage = block_invalidatepage;
@@ -70,7 +70,7 @@ static inline void truncate_partial_page
void cancel_dirty_page(struct page *page, unsigned int account_size)
{
if (TestClearPageDirty(page)) {
- struct address_space *mapping = page->mapping;
+ struct address_space *mapping = page_mapping_cache(page);
if (mapping && mapping_cap_account_dirty(mapping)) {
dec_zone_page_state(page, NR_FILE_DIRTY);
dec_bdi_stat(mapping->backing_dev_info,
@@ -95,7 +95,7 @@ EXPORT_SYMBOL(cancel_dirty_page);
static void
truncate_complete_page(struct address_space *mapping, struct page *page)
{
- if (page->mapping != mapping)
+ if (!is_page_consistent(page, mapping))
return;
cancel_dirty_page(page, PAGE_CACHE_SIZE);
@@ -122,7 +122,7 @@ invalidate_complete_page(struct address_
{
int ret;
- if (page->mapping != mapping)
+ if (!is_page_consistent(page, mapping))
return 0;
if (PagePrivate(page) && !try_to_release_page(page, 0))
@@ -344,7 +344,7 @@ EXPORT_SYMBOL(invalidate_mapping_pages);
static int
invalidate_complete_page2(struct address_space *mapping, struct page *page)
{
- if (page->mapping != mapping)
+ if (!is_page_consistent(page, mapping))
return 0;
if (PagePrivate(page) && !try_to_release_page(page, GFP_KERNEL))
@@ -369,7 +369,8 @@ static int do_launder_page(struct addres
{
if (!PageDirty(page))
return 0;
- if (page->mapping != mapping || mapping->a_ops->launder_page == NULL)
+ if (!is_page_consistent(page, mapping) ||
+ mapping->a_ops->launder_page == NULL)
return 0;
return mapping->a_ops->launder_page(page);
}
@@ -405,7 +406,7 @@ int invalidate_inode_pages2_range(struct
pgoff_t page_index;
lock_page(page);
- if (page->mapping != mapping) {
+ if (!is_page_consistent(page, mapping)) {
unlock_page(page);
continue;
}
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* [RFC][PATCH] page->mapping clarification [3/3] changes in /fs generic
2007-09-19 7:43 [RFC][PATCH] page->mapping clarification [1/3] base functions KAMEZAWA Hiroyuki
2007-09-19 7:44 ` [RFC][PATCH] page->mapping clarification [2/3] changes in /mm KAMEZAWA Hiroyuki
@ 2007-09-19 7:46 ` KAMEZAWA Hiroyuki
2007-09-20 18:26 ` [RFC][PATCH] page->mapping clarification [1/3] base functions Christoph Lameter
2007-09-21 11:48 ` Peter Zijlstra
3 siblings, 0 replies; 11+ messages in thread
From: KAMEZAWA Hiroyuki @ 2007-09-19 7:46 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: LKML, linux-mm, Andrew Morton, nickpiggin, ricknu-0
Make use of page-cache.h in fs-generic layer.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
fs/buffer.c | 43 ++++++++++++++++++++++---------------------
fs/fs-writeback.c | 2 +-
fs/libfs.c | 2 +-
fs/mpage.c | 13 +++++++------
4 files changed, 31 insertions(+), 29 deletions(-)
Index: linux-2.6.23-rc6-mm1/fs/buffer.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/fs/buffer.c
+++ linux-2.6.23-rc6-mm1/fs/buffer.c
@@ -467,7 +467,7 @@ static void end_buffer_async_write(struc
"I/O error on %s\n",
bdevname(bh->b_bdev, b));
}
- set_bit(AS_EIO, &page->mapping->flags);
+ set_bit(AS_EIO, &page_mapping_cache(page)->flags);
set_buffer_write_io_error(bh);
clear_buffer_uptodate(bh);
SetPageError(page);
@@ -678,7 +678,7 @@ void write_boundary_block(struct block_d
void mark_buffer_dirty_inode(struct buffer_head *bh, struct inode *inode)
{
struct address_space *mapping = inode->i_mapping;
- struct address_space *buffer_mapping = bh->b_page->mapping;
+ struct address_space *buffer_mapping = page_mapping_cache(bh->b_page);
mark_buffer_dirty(bh);
if (!mapping->assoc_mapping) {
@@ -713,7 +713,7 @@ static int __set_page_dirty(struct page
return 0;
write_lock_irq(&mapping->tree_lock);
- if (page->mapping) { /* Race with truncate? */
+ if (is_page_consistent(page, mapping)) {/* Race with truncate? */
WARN_ON_ONCE(warn && !PageUptodate(page));
if (mapping_cap_account_dirty(mapping)) {
@@ -1204,7 +1204,8 @@ void __bforget(struct buffer_head *bh)
{
clear_buffer_dirty(bh);
if (!list_empty(&bh->b_assoc_buffers)) {
- struct address_space *buffer_mapping = bh->b_page->mapping;
+ struct address_space *buffer_mapping;
+ buffer_mapping = page_mapping_cache(bh->b_page);
spin_lock(&buffer_mapping->private_lock);
list_del_init(&bh->b_assoc_buffers);
@@ -1544,7 +1545,7 @@ void create_empty_buffers(struct page *p
} while (bh);
tail->b_this_page = head;
- spin_lock(&page->mapping->private_lock);
+ spin_lock(&page_mapping_cache(page)->private_lock);
if (PageUptodate(page) || PageDirty(page)) {
bh = head;
do {
@@ -1556,7 +1557,7 @@ void create_empty_buffers(struct page *p
} while (bh != head);
}
attach_page_buffers(page, head);
- spin_unlock(&page->mapping->private_lock);
+ spin_unlock(&page_mapping_cache(page)->private_lock);
}
EXPORT_SYMBOL(create_empty_buffers);
@@ -1763,7 +1764,7 @@ recover:
} while ((bh = bh->b_this_page) != head);
SetPageError(page);
BUG_ON(PageWriteback(page));
- mapping_set_error(page->mapping, err);
+ mapping_set_error(page_mapping_cache(page), err);
set_page_writeback(page);
do {
struct buffer_head *next = bh->b_this_page;
@@ -2077,7 +2078,7 @@ EXPORT_SYMBOL(generic_write_end);
*/
int block_read_full_page(struct page *page, get_block_t *get_block)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
sector_t iblock, lblock;
struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
unsigned int blocksize;
@@ -2298,7 +2299,7 @@ out:
int block_prepare_write(struct page *page, unsigned from, unsigned to,
get_block_t *get_block)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
int err = __block_prepare_write(inode, page, from, to, get_block);
if (err)
ClearPageUptodate(page);
@@ -2307,7 +2308,7 @@ int block_prepare_write(struct page *pag
int block_commit_write(struct page *page, unsigned from, unsigned to)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
__block_commit_write(inode,page,from,to);
return 0;
}
@@ -2315,7 +2316,7 @@ int block_commit_write(struct page *page
int generic_commit_write(struct file *file, struct page *page,
unsigned from, unsigned to)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
__block_commit_write(inode,page,from,to);
/*
@@ -2355,7 +2356,7 @@ block_page_mkwrite(struct vm_area_struct
lock_page(page);
size = i_size_read(inode);
- if ((page->mapping != inode->i_mapping) ||
+ if (!is_page_consistent(page, inode->i_mapping) ||
(page_offset(page) > size)) {
/* page got truncated out from underneath us */
goto out_unlock;
@@ -2393,7 +2394,7 @@ static void end_buffer_read_nobh(struct
int nobh_prepare_write(struct page *page, unsigned from, unsigned to,
get_block_t *get_block)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
const unsigned blkbits = inode->i_blkbits;
const unsigned blocksize = 1 << blkbits;
struct buffer_head *head, *bh;
@@ -2507,7 +2508,7 @@ failed:
* the handling of potential IO errors during writeout would be hard
* (could try doing synchronous writeout, but what if that fails too?)
*/
- spin_lock(&page->mapping->private_lock);
+ spin_lock(&page_mapping_cache(page)->private_lock);
bh = head;
block_start = 0;
do {
@@ -2537,7 +2538,7 @@ next:
bh = bh->b_this_page;
} while (bh != head);
attach_page_buffers(page, head);
- spin_unlock(&page->mapping->private_lock);
+ spin_unlock(&page_mapping_cache(page)->private_lock);
return ret;
}
@@ -2550,7 +2551,7 @@ EXPORT_SYMBOL(nobh_prepare_write);
int nobh_commit_write(struct file *file, struct page *page,
unsigned from, unsigned to)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
if (page_has_buffers(page))
@@ -2574,7 +2575,7 @@ EXPORT_SYMBOL(nobh_commit_write);
int nobh_writepage(struct page *page, get_block_t *get_block,
struct writeback_control *wbc)
{
- struct inode * const inode = page->mapping->host;
+ struct inode * const inode = page_inode(page);
loff_t i_size = i_size_read(inode);
const pgoff_t end_index = i_size >> PAGE_CACHE_SHIFT;
unsigned offset;
@@ -2739,7 +2740,7 @@ out:
int block_write_full_page(struct page *page, get_block_t *get_block,
struct writeback_control *wbc)
{
- struct inode * const inode = page->mapping->host;
+ struct inode * const inode = page_inode(page);
loff_t i_size = i_size_read(inode);
const pgoff_t end_index = i_size >> PAGE_CACHE_SHIFT;
unsigned offset;
@@ -2968,8 +2969,8 @@ drop_buffers(struct page *page, struct b
bh = head;
do {
- if (buffer_write_io_error(bh) && page->mapping)
- set_bit(AS_EIO, &page->mapping->flags);
+ if (buffer_write_io_error(bh) && page_is_pagecache(page))
+ set_bit(AS_EIO, &page_mapping_cache(page)->flags);
if (buffer_busy(bh))
goto failed;
bh = bh->b_this_page;
@@ -2991,7 +2992,7 @@ failed:
int try_to_free_buffers(struct page *page)
{
- struct address_space * const mapping = page->mapping;
+ struct address_space * const mapping = page_mapping_cache(page);
struct buffer_head *buffers_to_free = NULL;
int ret = 0;
Index: linux-2.6.23-rc6-mm1/fs/fs-writeback.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/fs/fs-writeback.c
+++ linux-2.6.23-rc6-mm1/fs/fs-writeback.c
@@ -99,7 +99,7 @@ static void __check_dirty_inode_list(str
* the block-special inode (/dev/hda1) itself. And the ->dirtied_when field of
* the kernel-internal blockdev inode represents the dirtying time of the
* blockdev's pages. This is why for I_DIRTY_PAGES we always use
- * page->mapping->host, so the page-dirtying time is recorded in the internal
+ * page_inode(page), so the page-dirtying time is recorded in the internal
* blockdev inode.
*/
void __mark_inode_dirty(struct inode *inode, int flags)
Index: linux-2.6.23-rc6-mm1/fs/libfs.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/fs/libfs.c
+++ linux-2.6.23-rc6-mm1/fs/libfs.c
@@ -374,7 +374,7 @@ int simple_write_begin(struct file *file
static int simple_commit_write(struct file *file, struct page *page,
unsigned from, unsigned to)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
if (!PageUptodate(page))
Index: linux-2.6.23-rc6-mm1/fs/mpage.c
===================================================================
--- linux-2.6.23-rc6-mm1.orig/fs/mpage.c
+++ linux-2.6.23-rc6-mm1/fs/mpage.c
@@ -81,8 +81,9 @@ static int mpage_end_io_write(struct bio
if (!uptodate){
SetPageError(page);
- if (page->mapping)
- set_bit(AS_EIO, &page->mapping->flags);
+ if (page_is_pagecache(page))
+ set_bit(AS_EIO,
+ &page_mapping_cache(page)->flags);
}
end_page_writeback(page);
} while (bvec >= bio->bi_io_vec);
@@ -133,7 +134,7 @@ mpage_alloc(struct block_device *bdev,
static void
map_buffer_to_page(struct page *page, struct buffer_head *bh, int page_block)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
struct buffer_head *page_bh, *head;
int block = 0;
@@ -177,7 +178,7 @@ do_mpage_readpage(struct bio *bio, struc
sector_t *last_block_in_bio, struct buffer_head *map_bh,
unsigned long *first_logical_block, get_block_t get_block)
{
- struct inode *inode = page->mapping->host;
+ struct inode *inode = page_inode(page);
const unsigned blkbits = inode->i_blkbits;
const unsigned blocks_per_page = PAGE_CACHE_SIZE >> blkbits;
const unsigned blocksize = 1 << blkbits;
@@ -460,8 +461,8 @@ static int __mpage_writepage(struct page
{
struct mpage_data *mpd = data;
struct bio *bio = mpd->bio;
- struct address_space *mapping = page->mapping;
- struct inode *inode = page->mapping->host;
+ struct address_space *mapping = page_mapping_cache(page);
+ struct inode *inode = page_inode(page);
const unsigned blkbits = inode->i_blkbits;
unsigned long end_index;
const unsigned blocks_per_page = PAGE_CACHE_SIZE >> blkbits;
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-19 7:43 [RFC][PATCH] page->mapping clarification [1/3] base functions KAMEZAWA Hiroyuki
2007-09-19 7:44 ` [RFC][PATCH] page->mapping clarification [2/3] changes in /mm KAMEZAWA Hiroyuki
2007-09-19 7:46 ` [RFC][PATCH] page->mapping clarification [3/3] changes in /fs generic KAMEZAWA Hiroyuki
@ 2007-09-20 18:26 ` Christoph Lameter
2007-09-21 0:50 ` KAMEZAWA Hiroyuki
2007-09-21 11:48 ` Peter Zijlstra
3 siblings, 1 reply; 11+ messages in thread
From: Christoph Lameter @ 2007-09-20 18:26 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: LKML, linux-mm, Andrew Morton, nickpiggin, ricknu-0
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, 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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-20 18:26 ` [RFC][PATCH] page->mapping clarification [1/3] base functions Christoph Lameter
@ 2007-09-21 0:50 ` KAMEZAWA Hiroyuki
2007-09-21 17:02 ` Hugh Dickins
0 siblings, 1 reply; 11+ messages in thread
From: KAMEZAWA Hiroyuki @ 2007-09-21 0:50 UTC (permalink / raw)
To: Christoph Lameter; +Cc: LKML, linux-mm, Andrew Morton, nickpiggin, ricknu-0
On Thu, 20 Sep 2007 11:26:47 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:
> 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.
>
Honestly, I have 3 purposes, 2 for readability/clarificaton and 1 for my trial.
1. Clarify page cache <-> inode relationship before *new concept of page cache*,
yours or someone else's is introduced.
2. There are some places using PAGE_MAPPING_ANON directly. I don't want to see
following line in .c file.
==
anon_vma = (struct anon_vma *)(mapping - PAGE_MAPPING_ANON);
==
3. I want to *try* page->mapping overriding... store memory resource controller's
information in page->mapping. By this, memory controller doesn't enlarge sizeof
struct page. (works well in my small test.)
Before doing that, I have to hide page->mapping from direct access.
> > +/*
> > + * 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?
This is my first experience of using bool in Linux kernel.. :)
I know bool is not very widely used in Linux now but I tried it because
this function obviously returns yes or no, and C language supports bool as
_Bool now. If messy, I'll avoid using this in this time..
>
> > +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;
ok,
> > +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.
>
ok, thanks.
> > +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?
>
For clarify meaning of compareing page_mapping_cache() with mapping.
Does this reduce readability ?
Thank you for comments.
Regards,
-Kame
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-21 0:50 ` KAMEZAWA Hiroyuki
@ 2007-09-21 17:02 ` Hugh Dickins
2007-09-21 18:42 ` KAMEZAWA Hiroyuki
0 siblings, 1 reply; 11+ messages in thread
From: Hugh Dickins @ 2007-09-21 17:02 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: Christoph Lameter, LKML, linux-mm, Andrew Morton, nickpiggin,
ricknu-0, Magnus Damm
On Fri, 21 Sep 2007, KAMEZAWA Hiroyuki wrote:
> On Thu, 20 Sep 2007 11:26:47 -0700 (PDT)
> Christoph Lameter <clameter@sgi.com> wrote:
> >
> > I am still a bit confused as to what the benefit of this is.
> >
> Honestly, I have 3 purposes, 2 for readability/clarificaton and 1 for my trial.
>
> 1. Clarify page cache <-> inode relationship before *new concept of page cache*,
> yours or someone else's is introduced.
>
> 2. There are some places using PAGE_MAPPING_ANON directly. I don't want to see
> following line in .c file.
> ==
> anon_vma = (struct anon_vma *)(mapping - PAGE_MAPPING_ANON);
> ==
>
> 3. I want to *try* page->mapping overriding... store memory resource controller's
> information in page->mapping. By this, memory controller doesn't enlarge sizeof
> struct page. (works well in my small test.)
> Before doing that, I have to hide page->mapping from direct access.
My own vote (nothing more) would be for you to set this aside until
some future time when there aren't a dozen developers all trampling
over each other in this area.
They're invasive little changes affecting all filesystems, whereas what
we've done so far with page->mapping hasn't affected filesystems at all.
Purposes 1 and 2 don't score very high in my book (though I too regret
how mm/migrate.c copied that PAGE_MAPPING_ANON stuff from it's rightful
home in mm/rmap.c: maybe we should wrap that). There's no end to the
wrappers we can add, but they're not always helpful.
3: well, saving memory is good, but I think it could wait until some
other time, particularly since the memory controller isn't in yet.
Wouldn't it be easier to do something with page->lru than page->mapping?
Everybody is interested in page->mapping, not so many in page->lru.
(Though perhaps it wouldn't work out so well, since you don't need to
get uniquely from mapping to page, whereas you do from lru to page.)
If we were to attack page->mapping to save memory from struct page,
then we should consider Magnus Damm's idea too: he suggested it could
be replaced by a pointer to the radixtree slot (something else needed
in the anon case), from which "index" could be deduced via alignment
instead of keeping it in struct page (details to be filled in ...)
Of course, my particular prejudice is that I promised months ago to
free up the PG_swapcache bit by using a PAGE_MAPPING_SWAP bit instead.
That patch got buried while I tried to think up a suitable name for
a further page_mapping() variant that turned out to be needed - guess
I should look through your collection to see if I can steal one ;)
Beyond the unsatisfactory naming, that work has been long done
(and like PAGE_MAPPING_ANON, doesn't touch filesystems at all).
Or should I now leave PG_swapcache as is,
given your designs on page->mapping?
Hugh
p.s. Sorry to niggle, but next time, please say [PATCH 1/3] etc.
rather than [PATCH] Long Description [1/3], so it's easier to
sort the mail subjects by eye in limited columns - thanks.
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-21 17:02 ` Hugh Dickins
@ 2007-09-21 18:42 ` KAMEZAWA Hiroyuki
2007-09-26 19:31 ` Hugh Dickins
0 siblings, 1 reply; 11+ messages in thread
From: KAMEZAWA Hiroyuki @ 2007-09-21 18:42 UTC (permalink / raw)
To: Hugh Dickins
Cc: clameter, linux-kernel, linux-mm, akpm, nickpiggin, ricknu-0,
magnus.damm
On Fri, 21 Sep 2007 18:02:47 +0100 (BST)
Hugh Dickins <hugh@veritas.com> wrote:
> > 3. I want to *try* page->mapping overriding... store memory resource controller's
> > information in page->mapping. By this, memory controller doesn't enlarge sizeof
> > struct page. (works well in my small test.)
> > Before doing that, I have to hide page->mapping from direct access.
>
> My own vote (nothing more) would be for you to set this aside until
> some future time when there aren't a dozen developers all trampling
> over each other in this area.
>
> They're invasive little changes affecting all filesystems, whereas what
> we've done so far with page->mapping hasn't affected filesystems at all.
>
I found that each FS doesn't touch page->mapping so much as I expected.
(except for ReiserFS)
But ok, I admit changing this will confuse people.
> 3: well, saving memory is good, but I think it could wait until some
> other time, particularly since the memory controller isn't in yet.
>
Yes, if extra field in page struct is not hazard to push memory controller,
I don't have much motivation.
Because extra 8 bytes makes page struct to be 64 bytes(in 64bit), extra 8 bytes
is the last space, I think.
> If we were to attack page->mapping to save memory from struct page,
> then we should consider Magnus Damm's idea too: he suggested it could
> be replaced by a pointer to the radixtree slot (something else needed
> in the anon case), from which "index" could be deduced via alignment
> instead of keeping it in struct page (details to be filled in ...)
>
There is a bit difference. My purpose is "avoid making struct page larger",
not "making struct page smaller".
> Or should I now leave PG_swapcache as is,
> given your designs on page->mapping?
>
will conflict with my idea ?
==
http://marc.info/?l=linux-mm&m=118956492926821&w=2
==
Anyway, I'm not in hurry about this patch-set. I'll see what memory controller
will go. Other people seems to have an idea to implement
pfn <-> container_info_per_page function.
(But this kind of function is not welcomed always.)
Thank you for comments.
> p.s. Sorry to niggle, but next time, please say [PATCH 1/3] etc.
> rather than [PATCH] Long Description [1/3], so it's easier to
> sort the mail subjects by eye in limited columns - thanks.
>
sorry, I'll consider well next time.
Thanks,
-Kame
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-21 18:42 ` KAMEZAWA Hiroyuki
@ 2007-09-26 19:31 ` Hugh Dickins
2007-09-26 21:50 ` KAMEZAWA Hiroyuki
0 siblings, 1 reply; 11+ messages in thread
From: Hugh Dickins @ 2007-09-26 19:31 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: clameter, linux-kernel, linux-mm, akpm, nickpiggin, ricknu-0,
magnus.damm
On Sat, 22 Sep 2007, KAMEZAWA Hiroyuki wrote:
> On Fri, 21 Sep 2007 18:02:47 +0100 (BST)
> Hugh Dickins <hugh@veritas.com> wrote:
>
> > Or should I now leave PG_swapcache as is,
> > given your designs on page->mapping?
> >
> will conflict with my idea ?
> ==
> http://marc.info/?l=linux-mm&m=118956492926821&w=2
> ==
I asked because I had thought it would be a serious conflict: obviously
the patches as such would conflict quite a bit, but that's not serious,
one or the other just gets fixed up.
But now I don't see it - we both want to grab a further bit from the
low bits of the page->mapping pointer, you PAGE_MAPPING_INFO and me
PAGE_MAPPING_SWAP; but that's okay, so long as whoever is left using
bit (1<<2) is careful about the 32-bit case and remembers to put
__attribute__((aligned(sizeof(long long))))
on the declarations of struct address_space and struct anon_vma
and your struct page_mapping_info.
Would that waste a little memory? I think not with SLUB,
but perhaps with SLOB, which packs a little tighter.
Hugh
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-26 19:31 ` Hugh Dickins
@ 2007-09-26 21:50 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 11+ messages in thread
From: KAMEZAWA Hiroyuki @ 2007-09-26 21:50 UTC (permalink / raw)
To: Hugh Dickins
Cc: clameter, linux-kernel, linux-mm, akpm, nickpiggin, ricknu-0,
magnus.damm
On Wed, 26 Sep 2007 20:31:02 +0100 (BST)
Hugh Dickins <hugh@veritas.com> wrote:
> Would that waste a little memory? I think not with SLUB,
> but perhaps with SLOB, which packs a little tighter.
>
maybe just depends on the amount of used anon_vma and page_mapping_info etc...
I don't think a system which uses SLOB consumes such structs so much
as that memory-for-alignment is considered as "waste" of memory.
Anyway, I decided to go ahead with current container-info-per-page
implementation. If the size of page struct is problem at mainline inclusion
discussion, I'll be back.
Thanks,
-Kame
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-19 7:43 [RFC][PATCH] page->mapping clarification [1/3] base functions KAMEZAWA Hiroyuki
` (2 preceding siblings ...)
2007-09-20 18:26 ` [RFC][PATCH] page->mapping clarification [1/3] base functions Christoph Lameter
@ 2007-09-21 11:48 ` Peter Zijlstra
2007-09-21 15:06 ` KAMEZAWA Hiroyuki
3 siblings, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2007-09-21 11:48 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki; +Cc: LKML, linux-mm, Andrew Morton, nickpiggin, ricknu-0
On Wed, 19 Sep 2007 16:43:08 +0900 KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> A clarification of page <-> fs interface (page cache).
>
> At first, each FS has to access to struct page->mapping directly.
> But it's not just pointer. (we use special 1bit enconding for anon.)
>
> Although there is historical consensus that page->mapping points to its inode's
> address space, I think adding some neat helper functon is not bad.
>
> This patch adds page-cache.h which containes page<->address_space<->inode
> function which is required (used) by subsystems.
>
> 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
>
> Followings are moved
> * page_mapping() ... returns swapper_space or address_space a page is on.
> (from mm.h)
> * page_index() ... returns position of a page in its inode
> (from mm.h)
> * remove_mapping() ... a safe routine to remove page->mapping from page.
> (from swap.h)
I have two other functions that might want integration with this scheme:
page_file_mapping() ... returns backing address space
page_file_index() ... returns the index therein
They are identical to page_mapping_cache() and page_index() for
page cache pages, but they also work on swap cache pages.
That is, for swapcache pages they return:
page_file_mapping:
page_swap_info(page)->swap_file->f_mapping
page_file_index:
swp_offset((swp_offset_t)page_private(page))
When a filesystem uses these functions instead of page->mapping and
page->index, it allows passing swap cache pages into the regular
filesystem read/write paths.
This is useful for things like swap over NFS, where swap is backed by a
swapfile on a 'regular' filesystem.
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
2007-09-21 11:48 ` Peter Zijlstra
@ 2007-09-21 15:06 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 11+ messages in thread
From: KAMEZAWA Hiroyuki @ 2007-09-21 15:06 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: linux-kernel, linux-mm, akpm, nickpiggin, ricknu-0
On Fri, 21 Sep 2007 13:48:28 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> > Followings are moved
> > * page_mapping() ... returns swapper_space or address_space a page is on.
> > (from mm.h)
> > * page_index() ... returns position of a page in its inode
> > (from mm.h)
> > * remove_mapping() ... a safe routine to remove page->mapping from page.
> > (from swap.h)
>
> I have two other functions that might want integration with this scheme:
>
> page_file_mapping() ... returns backing address space
> page_file_index() ... returns the index therein
>
> They are identical to page_mapping_cache() and page_index() for
> page cache pages, but they also work on swap cache pages.
>
> That is, for swapcache pages they return:
>
> page_file_mapping:
> page_swap_info(page)->swap_file->f_mapping
>
> page_file_index:
> swp_offset((swp_offset_t)page_private(page))
>
> When a filesystem uses these functions instead of page->mapping and
> page->index, it allows passing swap cache pages into the regular
> filesystem read/write paths.
>
Oh,
> This is useful for things like swap over NFS, where swap is backed by a
> swapfile on a 'regular' filesystem.
>
Okay, I'll try to add them in the next set.
Thanks,
-Kame
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread