Re: [PATCH 2.6.17-rc1-mm3] add migratepage addresss space op to shmem

Re: [PATCH 2.6.17-rc1-mm3] add migratepage addresss space op to shmem

[Hugh Dickins]

[Sorry, I seem to have deleted the original, so destroyed threading]

On Thu, 20 Apr 2006, Lee Schermerhorn wrote:
> 
> In 2.6.16 through 2.6.17-rc1, shared memory mappings do not
> have a migratepage address space op.  Therefore, migrate_pages()
> falls back to default processing.  In this path, it will try to
> pageout() dirty pages.  Once a shared memory page has been migrated
> it becomes dirty, so migrate_pages() will try to page it out.  
> However, because the page count is 3 [cache + current + pte],
> pageout() will return PAGE_KEEP because is_page_cache_freeable()
> returns false.  This will abort all subsequent migrations.

So far as I can see, this problem is not at all peculiar to shmem
(aside from its greater likelihood of being found PageDirty): won't
that PageDirty pageout in migrate_pages always return PAGE_KEEP?
so as it stands, is pointless and misleading?

> This patch adds a migratepage address space op to shared memory
> segments to avoid taking the default path.  We use the "migrate_page()"
> function because it knows how to migrate dirty pages.  This allows
> shared memory segment pages to migrate, subject to other conditions
> such as # pte's referencing the page [page_mapcount(page)], when
> requested.  

While that's not wrong, wouldn't the right fix be something else?

> I think this is safe.  If we're migrating a shared memory page,
> then we found the page via a page table, so it must be in
> memory.

Yes, I agree: the isolate_lru_page while holding ptl keeps it sane.

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>

Re: [PATCH 2.6.17-rc1-mm3] add migratepage addresss space op to shmem

[Christoph Lameter]

On Mon, 24 Apr 2006, Hugh Dickins wrote:

> > In 2.6.16 through 2.6.17-rc1, shared memory mappings do not
> > have a migratepage address space op.  Therefore, migrate_pages()
> > falls back to default processing.  In this path, it will try to
> > pageout() dirty pages.  Once a shared memory page has been migrated
> > it becomes dirty, so migrate_pages() will try to page it out.  
> > However, because the page count is 3 [cache + current + pte],
> > pageout() will return PAGE_KEEP because is_page_cache_freeable()
> > returns false.  This will abort all subsequent migrations.
> 
> So far as I can see, this problem is not at all peculiar to shmem
> (aside from its greater likelihood of being found PageDirty): won't
> that PageDirty pageout in migrate_pages always return PAGE_KEEP?
> so as it stands, is pointless and misleading?

Yes, this wont work if we do not remove the ptes before calling 
pageout. A call to try_to_umap() is missing.

> > This patch adds a migratepage address space op to shared memory
> > segments to avoid taking the default path.  We use the "migrate_page()"
> > function because it knows how to migrate dirty pages.  This allows
> > shared memory segment pages to migrate, subject to other conditions
> > such as # pte's referencing the page [page_mapcount(page)], when
> > requested.  
> 
> While that's not wrong, wouldn't the right fix be something else?

His patch avoids going through the fallback functions and allows 
migrating dirty shmem pages without pageout. That is good.



Index: linux-2.6/mm/migrate.c
===================================================================
--- linux-2.6.orig/mm/migrate.c	2006-04-18 12:51:31.000000000 -0700
+++ linux-2.6/mm/migrate.c	2006-04-24 15:03:10.000000000 -0700
@@ -439,6 +439,11 @@ redo:
 			goto unlock_both;
                 }
 
+		if (try_to_unmap(page, 1) == SWAP_FAIL) {
+			rc = -EPERM;
+			goto unlock_both;
+		}
+
 		/*
 		 * Default handling if a filesystem does not provide
 		 * a migration function. We can only migrate clean

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

Re: [PATCH 2.6.17-rc1-mm3] add migratepage addresss space op to shmem

[Hugh Dickins]

On Mon, 24 Apr 2006, Christoph Lameter wrote:
> On Mon, 24 Apr 2006, Hugh Dickins wrote:
> > 
> > While that's not wrong, wouldn't the right fix be something else?
> 
> His patch avoids going through the fallback functions and allows 
> migrating dirty shmem pages without pageout. That is good.

True.

> Index: linux-2.6/mm/migrate.c
> ===================================================================
> --- linux-2.6.orig/mm/migrate.c	2006-04-18 12:51:31.000000000 -0700
> +++ linux-2.6/mm/migrate.c	2006-04-24 15:03:10.000000000 -0700
> @@ -439,6 +439,11 @@ redo:
>  			goto unlock_both;
>                  }
>  
> +		if (try_to_unmap(page, 1) == SWAP_FAIL) {
> +			rc = -EPERM;
> +			goto unlock_both;
> +		}
> +
>  		/*
>  		 * Default handling if a filesystem does not provide
>  		 * a migration function. We can only migrate clean

Perhaps.  But there seem to be altogether too many ways through this
code: this part of migrate_pages then starts to look rather like,
but not exactly like, swap_page.  Feels like it needs refactoring.

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>

Re: [PATCH 2.6.17-rc1-mm3] add migratepage addresss space op to shmem

[Christoph Lameter]

On Tue, 25 Apr 2006, Hugh Dickins wrote:

> Perhaps.  But there seem to be altogether too many ways through this
> code: this part of migrate_pages then starts to look rather like,
> but not exactly like, swap_page.  Feels like it needs refactoring.

I will have a look at it when the conference is over. Probably Thursday.

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

Re: [PATCH 2.6.17-rc1-mm3] add migratepage addresss space op to shmem

[Christoph Lameter]

On Tue, 25 Apr 2006, Hugh Dickins wrote:

> Perhaps.  But there seem to be altogether too many ways through this
> code: this part of migrate_pages then starts to look rather like,
> but not exactly like, swap_page.  Feels like it needs refactoring.

Actually the code migrates dirty pages because dirty information is 
available in the pte and not in the dirty bit of the page struct. 

The branch in question is never taken because at that point the dirty 
bit is not set in the the page struct.

Later we call migrate_page(). migrate page unmaps the ptes and transfers 
the dirty bit from the pte into the page struct. At that point we do not 
check the dirty bit again and therefore the page will be migrated without 
writeout. That is why the fallback mechanism passed the tests a couple of 
months back and thats also why Lee reported that migration works for shmem 
without a migratepage() method.

So the current problem is that the fallback path will migrate dirty pages 
without regard to page dirty state. This may cause a problem for 
filesystems that keep additional state for dirty pages and that do not 
define their a migration method (The only filesystems with migration 
methods are currently ext2,3 and xfs).

I need to do some restructuring of migrate_page_remove_references() in 
order to check the dirty state later. No need for pageout anymore. 
Pageout contains more swap specific code that is not needed anymore.

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