linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* UBIFS and page migration (take 3)
@ 2016-05-11 13:53 Richard Weinberger
  2016-05-11 13:53 ` [PATCH 1/2] mm: Export migrate_page_move_mapping and migrate_page_copy Richard Weinberger
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Richard Weinberger @ 2016-05-11 13:53 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: linux-mtd, linux-mm, linux-kernel, boris.brezillon,
	maxime.ripard, david, david, dedekind1, alex, akpm, sasha.levin,
	iamjoonsoo.kim, rvaswani, tony.luck, shailendra.capricorn,
	kirill.shutemov, hch, hughd, mgorman, vbabka

During page migrations UBIFS gets confused. We triggered this by using CMA
on two different targets.
It turned out that fallback_migrate_page() is not suitable for UBIFS as it
does not copy the PagePrivate flag. Non-trivial block based filesystems
do not notice since they can use buffer_migrate_page().
UBIFS is using this flag among with PageChecked to account free space.
One possible solution is implementing a ->migratepage() function in UBIFS
which does more or less the same as fallback_migrate_page() but also
copies PagePrivate. I'm not at all sure whether this is the way to go.
IMHO either page migration should not happen if ->migratepage() is not implement
or fallback_migrate_page() has to work for all filesystems.

Comments? Flames? :-)

Thanks,
//richard

[PATCH 1/2] mm: Export migrate_page_move_mapping and
[PATCH 2/2] UBIFS: Implement ->migratepage()

--
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] 15+ messages in thread

* [PATCH 1/2] mm: Export migrate_page_move_mapping and migrate_page_copy
  2016-05-11 13:53 UBIFS and page migration (take 3) Richard Weinberger
@ 2016-05-11 13:53 ` Richard Weinberger
  2016-05-11 13:53 ` [PATCH 2/2] UBIFS: Implement ->migratepage() Richard Weinberger
  2016-05-12 11:49 ` UBIFS and page migration (take 3) Christoph Hellwig
  2 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2016-05-11 13:53 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: linux-mtd, linux-mm, linux-kernel, boris.brezillon,
	maxime.ripard, david, david, dedekind1, alex, akpm, sasha.levin,
	iamjoonsoo.kim, rvaswani, tony.luck, shailendra.capricorn,
	kirill.shutemov, hch, hughd, mgorman, vbabka, Richard Weinberger

Export these symbols such that UBIFS can implement
->migratepage.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 mm/migrate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/migrate.c b/mm/migrate.c
index f9dfb18..bdf3410 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -431,6 +431,7 @@ int migrate_page_move_mapping(struct address_space *mapping,
 
 	return MIGRATEPAGE_SUCCESS;
 }
+EXPORT_SYMBOL(migrate_page_move_mapping);
 
 /*
  * The expected number of remaining references is the same as that
@@ -586,6 +587,7 @@ void migrate_page_copy(struct page *newpage, struct page *page)
 
 	mem_cgroup_migrate(page, newpage);
 }
+EXPORT_SYMBOL(migrate_page_copy);
 
 /************************************************************
  *                    Migration functions
-- 
2.7.3

--
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 related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] UBIFS: Implement ->migratepage()
  2016-05-11 13:53 UBIFS and page migration (take 3) Richard Weinberger
  2016-05-11 13:53 ` [PATCH 1/2] mm: Export migrate_page_move_mapping and migrate_page_copy Richard Weinberger
@ 2016-05-11 13:53 ` Richard Weinberger
  2016-05-12 10:47   ` Richard Weinberger
  2016-05-12 11:49 ` UBIFS and page migration (take 3) Christoph Hellwig
  2 siblings, 1 reply; 15+ messages in thread
From: Richard Weinberger @ 2016-05-11 13:53 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: linux-mtd, linux-mm, linux-kernel, boris.brezillon,
	maxime.ripard, david, david, dedekind1, alex, akpm, sasha.levin,
	iamjoonsoo.kim, rvaswani, tony.luck, shailendra.capricorn,
	kirill.shutemov, hch, hughd, mgorman, vbabka, Richard Weinberger

From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>

During page migrations UBIFS might get confused
and the following assert triggers:
[  213.480000] UBIFS assert failed in ubifs_set_page_dirty at 1451 (pid 436)
[  213.490000] CPU: 0 PID: 436 Comm: drm-stress-test Not tainted 4.4.4-00176-geaa802524636-dirty #1008
[  213.490000] Hardware name: Allwinner sun4i/sun5i Families
[  213.490000] [<c0015e70>] (unwind_backtrace) from [<c0012cdc>] (show_stack+0x10/0x14)
[  213.490000] [<c0012cdc>] (show_stack) from [<c02ad834>] (dump_stack+0x8c/0xa0)
[  213.490000] [<c02ad834>] (dump_stack) from [<c0236ee8>] (ubifs_set_page_dirty+0x44/0x50)
[  213.490000] [<c0236ee8>] (ubifs_set_page_dirty) from [<c00fa0bc>] (try_to_unmap_one+0x10c/0x3a8)
[  213.490000] [<c00fa0bc>] (try_to_unmap_one) from [<c00fadb4>] (rmap_walk+0xb4/0x290)
[  213.490000] [<c00fadb4>] (rmap_walk) from [<c00fb1bc>] (try_to_unmap+0x64/0x80)
[  213.490000] [<c00fb1bc>] (try_to_unmap) from [<c010dc28>] (migrate_pages+0x328/0x7a0)
[  213.490000] [<c010dc28>] (migrate_pages) from [<c00d0cb0>] (alloc_contig_range+0x168/0x2f4)
[  213.490000] [<c00d0cb0>] (alloc_contig_range) from [<c010ec00>] (cma_alloc+0x170/0x2c0)
[  213.490000] [<c010ec00>] (cma_alloc) from [<c001a958>] (__alloc_from_contiguous+0x38/0xd8)
[  213.490000] [<c001a958>] (__alloc_from_contiguous) from [<c001ad44>] (__dma_alloc+0x23c/0x274)
[  213.490000] [<c001ad44>] (__dma_alloc) from [<c001ae08>] (arm_dma_alloc+0x54/0x5c)
[  213.490000] [<c001ae08>] (arm_dma_alloc) from [<c035cecc>] (drm_gem_cma_create+0xb8/0xf0)
[  213.490000] [<c035cecc>] (drm_gem_cma_create) from [<c035cf20>] (drm_gem_cma_create_with_handle+0x1c/0xe8)
[  213.490000] [<c035cf20>] (drm_gem_cma_create_with_handle) from [<c035d088>] (drm_gem_cma_dumb_create+0x3c/0x48)
[  213.490000] [<c035d088>] (drm_gem_cma_dumb_create) from [<c0341ed8>] (drm_ioctl+0x12c/0x444)
[  213.490000] [<c0341ed8>] (drm_ioctl) from [<c0121adc>] (do_vfs_ioctl+0x3f4/0x614)
[  213.490000] [<c0121adc>] (do_vfs_ioctl) from [<c0121d30>] (SyS_ioctl+0x34/0x5c)
[  213.490000] [<c0121d30>] (SyS_ioctl) from [<c000f2c0>] (ret_fast_syscall+0x0/0x34)

UBIFS is using PagePrivate() which can have different meanings across
filesystems. Therefore the generic page migration code cannot handle this
case correctly.
We have to implement our own migration function which basically does a
plain copy but also duplicates the page private flag.
UBIFS is not a block device filesystem and cannot use buffer_migrate_page().

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
[rw: Massaged changelog, build fixes, etc...]
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/file.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 446753d..5e97bee 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -52,6 +52,7 @@
 #include "ubifs.h"
 #include <linux/mount.h>
 #include <linux/slab.h>
+#include <linux/migrate.h>
 
 static int read_block(struct inode *inode, void *addr, unsigned int block,
 		      struct ubifs_data_node *dn)
@@ -1452,6 +1453,24 @@ static int ubifs_set_page_dirty(struct page *page)
 	return ret;
 }
 
+static int ubifs_migrate_page(struct address_space *mapping,
+		struct page *newpage, struct page *page, enum migrate_mode mode)
+{
+	int rc;
+
+	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
+	if (rc != MIGRATEPAGE_SUCCESS)
+		return rc;
+
+	if (PagePrivate(page)) {
+		ClearPagePrivate(page);
+		SetPagePrivate(newpage);
+	}
+
+	migrate_page_copy(newpage, page);
+	return MIGRATEPAGE_SUCCESS;
+}
+
 static int ubifs_releasepage(struct page *page, gfp_t unused_gfp_flags)
 {
 	/*
@@ -1591,6 +1610,7 @@ const struct address_space_operations ubifs_file_address_operations = {
 	.write_end      = ubifs_write_end,
 	.invalidatepage = ubifs_invalidatepage,
 	.set_page_dirty = ubifs_set_page_dirty,
+	.migratepage	= ubifs_migrate_page,
 	.releasepage    = ubifs_releasepage,
 };
 
-- 
2.7.3

--
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 related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] UBIFS: Implement ->migratepage()
  2016-05-11 13:53 ` [PATCH 2/2] UBIFS: Implement ->migratepage() Richard Weinberger
@ 2016-05-12 10:47   ` Richard Weinberger
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2016-05-12 10:47 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: linux-mtd, linux-mm, linux-kernel, boris.brezillon,
	maxime.ripard, david, david, dedekind1, alex, akpm, sasha.levin,
	iamjoonsoo.kim, rvaswani, tony.luck, shailendra.capricorn,
	kirill.shutemov, hch, hughd, mgorman, vbabka, Richard Weinberger

From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>

During page migrations UBIFS might get confused
and the following assert triggers:
[  213.480000] UBIFS assert failed in ubifs_set_page_dirty at 1451 (pid 436)
[  213.490000] CPU: 0 PID: 436 Comm: drm-stress-test Not tainted 4.4.4-00176-geaa802524636-dirty #1008
[  213.490000] Hardware name: Allwinner sun4i/sun5i Families
[  213.490000] [<c0015e70>] (unwind_backtrace) from [<c0012cdc>] (show_stack+0x10/0x14)
[  213.490000] [<c0012cdc>] (show_stack) from [<c02ad834>] (dump_stack+0x8c/0xa0)
[  213.490000] [<c02ad834>] (dump_stack) from [<c0236ee8>] (ubifs_set_page_dirty+0x44/0x50)
[  213.490000] [<c0236ee8>] (ubifs_set_page_dirty) from [<c00fa0bc>] (try_to_unmap_one+0x10c/0x3a8)
[  213.490000] [<c00fa0bc>] (try_to_unmap_one) from [<c00fadb4>] (rmap_walk+0xb4/0x290)
[  213.490000] [<c00fadb4>] (rmap_walk) from [<c00fb1bc>] (try_to_unmap+0x64/0x80)
[  213.490000] [<c00fb1bc>] (try_to_unmap) from [<c010dc28>] (migrate_pages+0x328/0x7a0)
[  213.490000] [<c010dc28>] (migrate_pages) from [<c00d0cb0>] (alloc_contig_range+0x168/0x2f4)
[  213.490000] [<c00d0cb0>] (alloc_contig_range) from [<c010ec00>] (cma_alloc+0x170/0x2c0)
[  213.490000] [<c010ec00>] (cma_alloc) from [<c001a958>] (__alloc_from_contiguous+0x38/0xd8)
[  213.490000] [<c001a958>] (__alloc_from_contiguous) from [<c001ad44>] (__dma_alloc+0x23c/0x274)
[  213.490000] [<c001ad44>] (__dma_alloc) from [<c001ae08>] (arm_dma_alloc+0x54/0x5c)
[  213.490000] [<c001ae08>] (arm_dma_alloc) from [<c035cecc>] (drm_gem_cma_create+0xb8/0xf0)
[  213.490000] [<c035cecc>] (drm_gem_cma_create) from [<c035cf20>] (drm_gem_cma_create_with_handle+0x1c/0xe8)
[  213.490000] [<c035cf20>] (drm_gem_cma_create_with_handle) from [<c035d088>] (drm_gem_cma_dumb_create+0x3c/0x48)
[  213.490000] [<c035d088>] (drm_gem_cma_dumb_create) from [<c0341ed8>] (drm_ioctl+0x12c/0x444)
[  213.490000] [<c0341ed8>] (drm_ioctl) from [<c0121adc>] (do_vfs_ioctl+0x3f4/0x614)
[  213.490000] [<c0121adc>] (do_vfs_ioctl) from [<c0121d30>] (SyS_ioctl+0x34/0x5c)
[  213.490000] [<c0121d30>] (SyS_ioctl) from [<c000f2c0>] (ret_fast_syscall+0x0/0x34)

UBIFS is using PagePrivate() which can have different meanings across
filesystems. Therefore the generic page migration code cannot handle this
case correctly.
We have to implement our own migration function which basically does a
plain copy but also duplicates the page private flag.
UBIFS is not a block device filesystem and cannot use buffer_migrate_page().

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
[rw: Massaged changelog, build fixes, etc...]
Signed-off-by: Richard Weinberger <richard@nod.at>
---
Christ, sent the wrong patch without the !CONFIG_MIGRATION build fixes.

Thanks,
//richard
---
 fs/ubifs/file.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 446753d..5b5ec8d 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -52,6 +52,7 @@
 #include "ubifs.h"
 #include <linux/mount.h>
 #include <linux/slab.h>
+#include <linux/migrate.h>
 
 static int read_block(struct inode *inode, void *addr, unsigned int block,
 		      struct ubifs_data_node *dn)
@@ -1452,6 +1453,26 @@ static int ubifs_set_page_dirty(struct page *page)
 	return ret;
 }
 
+#ifdef CONFIG_MIGRATION
+static int ubifs_migrate_page(struct address_space *mapping,
+		struct page *newpage, struct page *page, enum migrate_mode mode)
+{
+	int rc;
+
+	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
+	if (rc != MIGRATEPAGE_SUCCESS)
+		return rc;
+
+	if (PagePrivate(page)) {
+		ClearPagePrivate(page);
+		SetPagePrivate(newpage);
+	}
+
+	migrate_page_copy(newpage, page);
+	return MIGRATEPAGE_SUCCESS;
+}
+#endif
+
 static int ubifs_releasepage(struct page *page, gfp_t unused_gfp_flags)
 {
 	/*
@@ -1591,6 +1612,9 @@ const struct address_space_operations ubifs_file_address_operations = {
 	.write_end      = ubifs_write_end,
 	.invalidatepage = ubifs_invalidatepage,
 	.set_page_dirty = ubifs_set_page_dirty,
+#ifdef CONFIG_MIGRATION
+	.migratepage	= ubifs_migrate_page,
+#endif
 	.releasepage    = ubifs_releasepage,
 };
 
-- 
2.7.3

--
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 related	[flat|nested] 15+ messages in thread

* Re: UBIFS and page migration (take 3)
  2016-05-11 13:53 UBIFS and page migration (take 3) Richard Weinberger
  2016-05-11 13:53 ` [PATCH 1/2] mm: Export migrate_page_move_mapping and migrate_page_copy Richard Weinberger
  2016-05-11 13:53 ` [PATCH 2/2] UBIFS: Implement ->migratepage() Richard Weinberger
@ 2016-05-12 11:49 ` Christoph Hellwig
  2016-05-16 12:44   ` Richard Weinberger
  2 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2016-05-12 11:49 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: linux-fsdevel, linux-mtd, linux-mm, linux-kernel,
	boris.brezillon, maxime.ripard, david, david, dedekind1, alex,
	akpm, sasha.levin, iamjoonsoo.kim, rvaswani, tony.luck,
	shailendra.capricorn, kirill.shutemov, hch, hughd, mgorman,
	vbabka

Hi Richard,

the series looks fine to me, but it fails to address the root cause:
that we have an inherently dangerous default for ->migratepage that
assumes that file systems are implemented a certain way.  I think the
series should also grow a third patch to remove the default and just
wire it up for the known good file systems, although we'd need some
input on what known good is.

Any idea what filesystems do get regular testing with code that's using
CMA? A good approximation might be those that use the bufer_head
based aops from fs/buffer.c

--
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] 15+ messages in thread

* Re: UBIFS and page migration (take 3)
  2016-05-12 11:49 ` UBIFS and page migration (take 3) Christoph Hellwig
@ 2016-05-16 12:44   ` Richard Weinberger
  2016-05-16 13:03     ` Vlastimil Babka
  2016-05-17  9:11     ` Christoph Hellwig
  0 siblings, 2 replies; 15+ messages in thread
From: Richard Weinberger @ 2016-05-16 12:44 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-fsdevel, linux-mtd, linux-mm, linux-kernel,
	boris.brezillon, maxime.ripard, david, david, dedekind1, alex,
	akpm, sasha.levin, iamjoonsoo.kim, rvaswani, tony.luck,
	shailendra.capricorn, kirill.shutemov, hughd, mgorman, vbabka

Christoph,

Am 12.05.2016 um 13:49 schrieb Christoph Hellwig:
> Hi Richard,
> 
> the series looks fine to me, but it fails to address the root cause:

Is this a Reviewed-by? :-)

> that we have an inherently dangerous default for ->migratepage that
> assumes that file systems are implemented a certain way.  I think the
> series should also grow a third patch to remove the default and just
> wire it up for the known good file systems, although we'd need some
> input on what known good is.
>
> Any idea what filesystems do get regular testing with code that's using
> CMA? A good approximation might be those that use the bufer_head
> based aops from fs/buffer.c

No idea how much is being tested.
I fear most issues are unknown. At least for UBIFS it took
years to get aware of the issue.
Thanks again to Maxime and Boris for providing a reproducer.

There are two classes of issues:
a) filesystems that use buffer_migrate_page() but shouldn't
b) filesystems that don't implement ->migratepage() and fallback_migrate_page()
   is not suitable.

As starter we could kill the automatic assignment of fallback_migrate_page() and
non-buffer_head filesystems need to figure out whether fallback_migrate_page()
is suitable or not.
UBIFS found out the hard way. ;-\

MM folks, do we have a way to force page migration?
Maybe we can create a generic stress test.

Thanks,
//richard

--
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] 15+ messages in thread

* Re: UBIFS and page migration (take 3)
  2016-05-16 12:44   ` Richard Weinberger
@ 2016-05-16 13:03     ` Vlastimil Babka
  2016-05-17  9:12       ` Christoph Hellwig
  2016-05-17 11:27       ` Richard Weinberger
  2016-05-17  9:11     ` Christoph Hellwig
  1 sibling, 2 replies; 15+ messages in thread
From: Vlastimil Babka @ 2016-05-16 13:03 UTC (permalink / raw)
  To: Richard Weinberger, Christoph Hellwig
  Cc: linux-fsdevel, linux-mtd, linux-mm, linux-kernel,
	boris.brezillon, maxime.ripard, david, david, dedekind1, alex,
	akpm, sasha.levin, iamjoonsoo.kim, rvaswani, tony.luck,
	shailendra.capricorn, kirill.shutemov, hughd, mgorman

On 05/16/2016 02:44 PM, Richard Weinberger wrote:
> MM folks, do we have a way to force page migration?

On NUMA we have migrate_pages(2).

> Maybe we can create a generic stress test.
>
> Thanks,
> //richard
>
> --
> 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>
>

--
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] 15+ messages in thread

* Re: UBIFS and page migration (take 3)
  2016-05-16 12:44   ` Richard Weinberger
  2016-05-16 13:03     ` Vlastimil Babka
@ 2016-05-17  9:11     ` Christoph Hellwig
  1 sibling, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2016-05-17  9:11 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Christoph Hellwig, linux-fsdevel, linux-mtd, linux-mm,
	linux-kernel, boris.brezillon, maxime.ripard, david, david,
	dedekind1, alex, akpm, sasha.levin, iamjoonsoo.kim, rvaswani,
	tony.luck, shailendra.capricorn, kirill.shutemov, hughd, mgorman,
	vbabka

On Mon, May 16, 2016 at 02:44:49PM +0200, Richard Weinberger wrote:
> Is this a Reviewed-by? :-)

I don't know the code well enough to feel qualified for a review.  But
you get my:

Acked-by: Christoph Hellwig <hch@lst.de>

> There are two classes of issues:
> a) filesystems that use buffer_migrate_page() but shouldn't
> b) filesystems that don't implement ->migratepage() and fallback_migrate_page()
>    is not suitable.
> 
> As starter we could kill the automatic assignment of fallback_migrate_page() and
> non-buffer_head filesystems need to figure out whether fallback_migrate_page()
> is suitable or not.
> UBIFS found out the hard way. ;-\

Yes, I think this would be a good start.

--
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] 15+ messages in thread

* Re: UBIFS and page migration (take 3)
  2016-05-16 13:03     ` Vlastimil Babka
@ 2016-05-17  9:12       ` Christoph Hellwig
  2016-05-17 11:27       ` Richard Weinberger
  1 sibling, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2016-05-17  9:12 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Richard Weinberger, Christoph Hellwig, linux-fsdevel, linux-mtd,
	linux-mm, linux-kernel, boris.brezillon, maxime.ripard, david,
	david, dedekind1, alex, akpm, sasha.levin, iamjoonsoo.kim,
	rvaswani, tony.luck, shailendra.capricorn, kirill.shutemov,
	hughd, mgorman

On Mon, May 16, 2016 at 03:03:55PM +0200, Vlastimil Babka wrote:
> On 05/16/2016 02:44 PM, Richard Weinberger wrote:
> >MM folks, do we have a way to force page migration?
> 
> On NUMA we have migrate_pages(2).

Do we have existing sets of it?  Otherwise it would be good to find
a way to wire it up for xfstests so that we enforce migratepage
is called.  Once I get some time I'll see how often we end up
calling migratepage for a normal QA run.

--
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] 15+ messages in thread

* Re: UBIFS and page migration (take 3)
  2016-05-16 13:03     ` Vlastimil Babka
  2016-05-17  9:12       ` Christoph Hellwig
@ 2016-05-17 11:27       ` Richard Weinberger
  2016-05-18  9:03         ` Richard Weinberger
  1 sibling, 1 reply; 15+ messages in thread
From: Richard Weinberger @ 2016-05-17 11:27 UTC (permalink / raw)
  To: Vlastimil Babka, Christoph Hellwig
  Cc: linux-fsdevel, linux-mtd, linux-mm, linux-kernel,
	boris.brezillon, maxime.ripard, david, david, dedekind1, alex,
	akpm, sasha.levin, iamjoonsoo.kim, rvaswani, tony.luck,
	shailendra.capricorn, kirill.shutemov, hughd, mgorman

Vlastimil,

Am 16.05.2016 um 15:03 schrieb Vlastimil Babka:
> On 05/16/2016 02:44 PM, Richard Weinberger wrote:
>> MM folks, do we have a way to force page migration?
> 
> On NUMA we have migrate_pages(2).

Doesn't this only migrate process (user) pages?
AFAIK we need a way to force migration of pages which
are in the page cache.

*confused*,
//richard

--
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] 15+ messages in thread

* Re: UBIFS and page migration (take 3)
  2016-05-17 11:27       ` Richard Weinberger
@ 2016-05-18  9:03         ` Richard Weinberger
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2016-05-18  9:03 UTC (permalink / raw)
  To: Vlastimil Babka, Christoph Hellwig
  Cc: linux-fsdevel, linux-mtd, linux-mm, linux-kernel,
	boris.brezillon, maxime.ripard, david, david, dedekind1, alex,
	akpm, sasha.levin, iamjoonsoo.kim, rvaswani, tony.luck,
	shailendra.capricorn, kirill.shutemov, hughd, mgorman

Am 17.05.2016 um 13:27 schrieb Richard Weinberger:
> Vlastimil,
> 
> Am 16.05.2016 um 15:03 schrieb Vlastimil Babka:
>> On 05/16/2016 02:44 PM, Richard Weinberger wrote:
>>> MM folks, do we have a way to force page migration?
>>
>> On NUMA we have migrate_pages(2).
> 
> Doesn't this only migrate process (user) pages?
> AFAIK we need a way to force migration of pages which
> are in the page cache.

While migrate_pages(2) does not seem to hit the ->migratepage()
function, echo 1 > /proc/sys/vm/compact_memory does.
Thanks Vlastimil for pointing me in the right direction!

Just reproduce the issue on UBIFS in a KVM guest and a
simulated NAND. :-)
Let's see whether I can come up with a generic test.

Thanks,
//richard

--
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] 15+ messages in thread

* Re: [PATCH 2/2] UBIFS: Implement ->migratepage()
  2016-03-31 21:58 ` [PATCH 2/2] UBIFS: Implement ->migratepage() Richard Weinberger
  2016-04-01 10:14   ` Vlastimil Babka
@ 2016-04-03  0:13   ` kbuild test robot
  1 sibling, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2016-04-03  0:13 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: kbuild-all, linux-fsdevel, linux-mtd, linux-mm, linux-kernel,
	boris.brezillon, maxime.ripard, david, david, dedekind1, alex,
	akpm, sasha.levin, iamjoonsoo.kim, rvaswani, tony.luck,
	shailendra.capricorn, kirill.shutemov, hch, hughd, mgorman,
	vbabka, Richard Weinberger

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

Hi Kirill,

[auto build test ERROR on v4.6-rc1]
[also build test ERROR on next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Richard-Weinberger/mm-Export-migrate_page_move_mapping-and-migrate_page_copy/20160401-060041
config: xtensa-audio_kc705_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   fs/ubifs/file.c: In function 'ubifs_migrate_page':
>> fs/ubifs/file.c:1461:2: error: implicit declaration of function 'migrate_page_move_mapping' [-Werror=implicit-function-declaration]
     rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
     ^
   cc1: some warnings being treated as errors

vim +/migrate_page_move_mapping +1461 fs/ubifs/file.c

  1455	
  1456	static int ubifs_migrate_page(struct address_space *mapping,
  1457			struct page *newpage, struct page *page, enum migrate_mode mode)
  1458	{
  1459		int rc;
  1460	
> 1461		rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
  1462		if (rc != MIGRATEPAGE_SUCCESS)
  1463			return rc;
  1464	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 12169 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] UBIFS: Implement ->migratepage()
  2016-04-01 10:14   ` Vlastimil Babka
@ 2016-04-01 11:21     ` Richard Weinberger
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2016-04-01 11:21 UTC (permalink / raw)
  To: Vlastimil Babka, linux-fsdevel
  Cc: linux-mtd, linux-mm, linux-kernel, boris.brezillon,
	maxime.ripard, david, david, dedekind1, alex, akpm, sasha.levin,
	iamjoonsoo.kim, rvaswani, tony.luck, shailendra.capricorn,
	kirill.shutemov, hch, hughd, mgorman

Am 01.04.2016 um 12:14 schrieb Vlastimil Babka:
> On 03/31/2016 11:58 PM, Richard Weinberger wrote:
>> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>>
>> During page migrations UBIFS might get confused
>> and the following assert triggers:
>> UBIFS assert failed in ubifs_set_page_dirty at 1451 (pid 436)
> 
> It would be useful to have the full trace in changelog.

Oh. Yes.

>> UBIFS is using PagePrivate() which can have different meanings across
>> filesystems. Therefore the generic page migration code cannot handle this
>> case correctly.
>> We have to implement our own migration function which basically does a
>> plain copy but also duplicates the page private flag.
>> UBIFS is not a block device filesystem and cannot use buffer_migrate_page().
>>
>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> [rw: Massaged changelog]
>> Signed-off-by: Richard Weinberger <richard@nod.at>
> 
> Stable?

Yep. But first I'd like to clarify if this approach is really the way to go.
It is also not clear to me whether this issue was always the case or if
a recently introduced change in MM uncovered it...
Blindly applying to all stable versions is risky.

Thanks,
//richard

--
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] 15+ messages in thread

* Re: [PATCH 2/2] UBIFS: Implement ->migratepage()
  2016-03-31 21:58 ` [PATCH 2/2] UBIFS: Implement ->migratepage() Richard Weinberger
@ 2016-04-01 10:14   ` Vlastimil Babka
  2016-04-01 11:21     ` Richard Weinberger
  2016-04-03  0:13   ` kbuild test robot
  1 sibling, 1 reply; 15+ messages in thread
From: Vlastimil Babka @ 2016-04-01 10:14 UTC (permalink / raw)
  To: Richard Weinberger, linux-fsdevel
  Cc: linux-mtd, linux-mm, linux-kernel, boris.brezillon,
	maxime.ripard, david, david, dedekind1, alex, akpm, sasha.levin,
	iamjoonsoo.kim, rvaswani, tony.luck, shailendra.capricorn,
	kirill.shutemov, hch, hughd, mgorman

On 03/31/2016 11:58 PM, Richard Weinberger wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> During page migrations UBIFS might get confused
> and the following assert triggers:
> UBIFS assert failed in ubifs_set_page_dirty at 1451 (pid 436)

It would be useful to have the full trace in changelog.

> UBIFS is using PagePrivate() which can have different meanings across
> filesystems. Therefore the generic page migration code cannot handle this
> case correctly.
> We have to implement our own migration function which basically does a
> plain copy but also duplicates the page private flag.
> UBIFS is not a block device filesystem and cannot use buffer_migrate_page().
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [rw: Massaged changelog]
> Signed-off-by: Richard Weinberger <richard@nod.at>

Stable?

> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>   fs/ubifs/file.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 065c88f..5eea5f5 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -52,6 +52,7 @@
>   #include "ubifs.h"
>   #include <linux/mount.h>
>   #include <linux/slab.h>
> +#include <linux/migrate.h>
>
>   static int read_block(struct inode *inode, void *addr, unsigned int block,
>   		      struct ubifs_data_node *dn)
> @@ -1452,6 +1453,24 @@ static int ubifs_set_page_dirty(struct page *page)
>   	return ret;
>   }
>
> +static int ubifs_migrate_page(struct address_space *mapping,
> +		struct page *newpage, struct page *page, enum migrate_mode mode)
> +{
> +	int rc;
> +
> +	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
> +	if (rc != MIGRATEPAGE_SUCCESS)
> +		return rc;
> +
> +	if (PagePrivate(page)) {
> +		ClearPagePrivate(page);
> +		SetPagePrivate(newpage);
> +	}
> +
> +	migrate_page_copy(newpage, page);
> +	return MIGRATEPAGE_SUCCESS;
> +}
> +
>   static int ubifs_releasepage(struct page *page, gfp_t unused_gfp_flags)
>   {
>   	/*
> @@ -1591,6 +1610,7 @@ const struct address_space_operations ubifs_file_address_operations = {
>   	.write_end      = ubifs_write_end,
>   	.invalidatepage = ubifs_invalidatepage,
>   	.set_page_dirty = ubifs_set_page_dirty,
> +	.migratepage	= ubifs_migrate_page,
>   	.releasepage    = ubifs_releasepage,
>   };
>
>

--
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] 15+ messages in thread

* [PATCH 2/2] UBIFS: Implement ->migratepage()
  2016-03-31 21:58 UBIFS and page migration (take 2) Richard Weinberger
@ 2016-03-31 21:58 ` Richard Weinberger
  2016-04-01 10:14   ` Vlastimil Babka
  2016-04-03  0:13   ` kbuild test robot
  0 siblings, 2 replies; 15+ messages in thread
From: Richard Weinberger @ 2016-03-31 21:58 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: linux-mtd, linux-mm, linux-kernel, boris.brezillon,
	maxime.ripard, david, david, dedekind1, alex, akpm, sasha.levin,
	iamjoonsoo.kim, rvaswani, tony.luck, shailendra.capricorn,
	kirill.shutemov, hch, hughd, mgorman, vbabka, Richard Weinberger

From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>

During page migrations UBIFS might get confused
and the following assert triggers:
UBIFS assert failed in ubifs_set_page_dirty at 1451 (pid 436)

UBIFS is using PagePrivate() which can have different meanings across
filesystems. Therefore the generic page migration code cannot handle this
case correctly.
We have to implement our own migration function which basically does a
plain copy but also duplicates the page private flag.
UBIFS is not a block device filesystem and cannot use buffer_migrate_page().

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
[rw: Massaged changelog]
Signed-off-by: Richard Weinberger <richard@nod.at>

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/file.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 065c88f..5eea5f5 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -52,6 +52,7 @@
 #include "ubifs.h"
 #include <linux/mount.h>
 #include <linux/slab.h>
+#include <linux/migrate.h>
 
 static int read_block(struct inode *inode, void *addr, unsigned int block,
 		      struct ubifs_data_node *dn)
@@ -1452,6 +1453,24 @@ static int ubifs_set_page_dirty(struct page *page)
 	return ret;
 }
 
+static int ubifs_migrate_page(struct address_space *mapping,
+		struct page *newpage, struct page *page, enum migrate_mode mode)
+{
+	int rc;
+
+	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
+	if (rc != MIGRATEPAGE_SUCCESS)
+		return rc;
+
+	if (PagePrivate(page)) {
+		ClearPagePrivate(page);
+		SetPagePrivate(newpage);
+	}
+
+	migrate_page_copy(newpage, page);
+	return MIGRATEPAGE_SUCCESS;
+}
+
 static int ubifs_releasepage(struct page *page, gfp_t unused_gfp_flags)
 {
 	/*
@@ -1591,6 +1610,7 @@ const struct address_space_operations ubifs_file_address_operations = {
 	.write_end      = ubifs_write_end,
 	.invalidatepage = ubifs_invalidatepage,
 	.set_page_dirty = ubifs_set_page_dirty,
+	.migratepage	= ubifs_migrate_page,
 	.releasepage    = ubifs_releasepage,
 };
 
-- 
1.8.4.5

--
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 related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-05-18  9:03 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-11 13:53 UBIFS and page migration (take 3) Richard Weinberger
2016-05-11 13:53 ` [PATCH 1/2] mm: Export migrate_page_move_mapping and migrate_page_copy Richard Weinberger
2016-05-11 13:53 ` [PATCH 2/2] UBIFS: Implement ->migratepage() Richard Weinberger
2016-05-12 10:47   ` Richard Weinberger
2016-05-12 11:49 ` UBIFS and page migration (take 3) Christoph Hellwig
2016-05-16 12:44   ` Richard Weinberger
2016-05-16 13:03     ` Vlastimil Babka
2016-05-17  9:12       ` Christoph Hellwig
2016-05-17 11:27       ` Richard Weinberger
2016-05-18  9:03         ` Richard Weinberger
2016-05-17  9:11     ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2016-03-31 21:58 UBIFS and page migration (take 2) Richard Weinberger
2016-03-31 21:58 ` [PATCH 2/2] UBIFS: Implement ->migratepage() Richard Weinberger
2016-04-01 10:14   ` Vlastimil Babka
2016-04-01 11:21     ` Richard Weinberger
2016-04-03  0:13   ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).