* Re: + compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch added to -mm tree
2017-03-09 7:58 ` + compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch added to -mm tree Vlastimil Babka
@ 2017-03-09 11:50 ` zhouxianrong
2017-03-09 12:15 ` Jan Kara
1 sibling, 0 replies; 4+ messages in thread
From: zhouxianrong @ 2017-03-09 11:50 UTC (permalink / raw)
To: Vlastimil Babka, akpm, Mi.Sophia.Wang, hannes, kirill.shutemov,
mgorman, minchan, viro, weidu.du, won.ho.park, zhangshiming5,
zhouxiaoyan1, zhouxiyu, mm-commits, Jan Kara, Hugh Dickins,
Linux-FSDevel, LKML
On 2017/3/9 15:58, Vlastimil Babka wrote:
> On 03/09/2017 12:55 AM, akpm@linux-foundation.org wrote:
>>
>> The patch titled
>> Subject: compaction: add def_blk_aops migrate function for memory compaction
>> has been added to the -mm tree. Its filename is
>> compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
>>
>> This patch should soon appear at
>> http://ozlabs.org/~akpm/mmots/broken-out/compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
>> and later at
>> http://ozlabs.org/~akpm/mmotm/broken-out/compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
>>
>> Before you just go and hit "reply", please:
>> a) Consider who else should be cc'ed
>> b) Prefer to cc a suitable mailing list as well
>> c) Ideally: find the original patch on the mailing list and do a
>> reply-to-all to that, adding suitable additional cc's
>>
>> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>>
>> The -mm tree is included into linux-next and is updated
>> there every 3-4 working days
>>
>> ------------------------------------------------------
>> From: zhouxianrong <zhouxianrong@huawei.com>
>> Subject: compaction: add def_blk_aops migrate function for memory compaction
>
> That's not really a mm/compaction patch, but a block layer/migration patch. I
> don't know internals of those so well, so I added some CC's.
>
>> The reason for doing this is based on two factors.
>>
>> 1. larg file read/write operations with order 0 can fragmentize
>> memory rapidly.
>>
>> 2. when a special filesystem does not supply migratepage callback,
>> kernel would fallback to default function fallback_migrate_page.
>> but fallback_migrate_page could not migrate diry page nicely;
>> specially kcompactd with MIGRATE_SYNC_LIGHT could not migrate
>> diry pages due to this until clear_page_dirty_for_io in some
>> procedure. i think it is not suitable here in this scenario.
>> for dirty pages we should migrate it rather than skip or writeout
>> it in kcomapctd with MIGRATE_SYNC_LIGHT. i think this problem is
>> for all filesystem without migratepage not only for block device fs.
>>
>> So for compaction under large file writing supply migratepage for
>> def_blk_aops.
>
> Is this really safe to do? buffer_migrate_page() has some assumptions listed in
> its comment (and maybe more that are not listed). Do we know it's safe to use it
> for all def_blk_aops users?
I could not find out differences for different disks in block device filesystem;
they should behave consistently in block device filesystem layer.
for a page of file /dev/block/xxx, when we migrate it, i think it has no difference
just like ext4 file migration.
but i dare not to say yes. i hope more peoples give their suggestions.
>
>> Link: http://lkml.kernel.org/r/1488937915-78955-1-git-send-email-zhouxianrong@huawei.com
>> Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
>> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: Minchan Kim <minchan@kernel.org>
>> Cc: Mel Gorman <mgorman@techsingularity.net>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>> Cc: <Mi.Sophia.Wang@huawei.com>
>> Cc: <zhouxiyu@huawei.com>
>> Cc: <weidu.du@huawei.com>
>> Cc: <zhangshiming5@huawei.com>
>> Cc: <won.ho.park@huawei.com>
>> Cc: <zhouxiaoyan1@huawei.com>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> ---
>>
>> fs/block_dev.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff -puN fs/block_dev.c~compaction-add-def_blk_aops-migrate-function-for-memory-compaction fs/block_dev.c
>> --- a/fs/block_dev.c~compaction-add-def_blk_aops-migrate-function-for-memory-compaction
>> +++ a/fs/block_dev.c
>> @@ -2064,6 +2064,9 @@ static const struct address_space_operat
>> .releasepage = blkdev_releasepage,
>> .direct_IO = blkdev_direct_IO,
>> .is_dirty_writeback = buffer_check_dirty_writeback,
>> +#ifdef CONFIG_MIGRATION
>> + .migratepage = buffer_migrate_page,
>> +#endif
>> };
>>
>> #define BLKDEV_FALLOC_FL_SUPPORTED \
>> _
>>
>> Patches currently in -mm which might be from zhouxianrong@huawei.com are
>>
>> compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch added to -mm tree
2017-03-09 7:58 ` + compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch added to -mm tree Vlastimil Babka
2017-03-09 11:50 ` zhouxianrong
@ 2017-03-09 12:15 ` Jan Kara
2017-03-09 13:19 ` Michal Hocko
1 sibling, 1 reply; 4+ messages in thread
From: Jan Kara @ 2017-03-09 12:15 UTC (permalink / raw)
To: Vlastimil Babka
Cc: akpm, zhouxianrong, Mi.Sophia.Wang, hannes, kirill.shutemov,
mgorman, minchan, viro, weidu.du, won.ho.park, zhangshiming5,
zhouxiaoyan1, zhouxiyu, mm-commits, Jan Kara, Hugh Dickins,
Linux-FSDevel, LKML
On Thu 09-03-17 08:58:45, Vlastimil Babka wrote:
> On 03/09/2017 12:55 AM, akpm@linux-foundation.org wrote:
> >
> > The patch titled
> > Subject: compaction: add def_blk_aops migrate function for memory compaction
> > has been added to the -mm tree. Its filename is
> > compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
> >
> > This patch should soon appear at
> > http://ozlabs.org/~akpm/mmots/broken-out/compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
> > and later at
> > http://ozlabs.org/~akpm/mmotm/broken-out/compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
> >
> > Before you just go and hit "reply", please:
> > a) Consider who else should be cc'ed
> > b) Prefer to cc a suitable mailing list as well
> > c) Ideally: find the original patch on the mailing list and do a
> > reply-to-all to that, adding suitable additional cc's
> >
> > *** Remember to use Documentation/SubmitChecklist when testing your code ***
> >
> > The -mm tree is included into linux-next and is updated
> > there every 3-4 working days
> >
> > ------------------------------------------------------
> > From: zhouxianrong <zhouxianrong@huawei.com>
> > Subject: compaction: add def_blk_aops migrate function for memory compaction
>
> That's not really a mm/compaction patch, but a block layer/migration patch. I
> don't know internals of those so well, so I added some CC's.
Thanks!
> > The reason for doing this is based on two factors.
> >
> > 1. larg file read/write operations with order 0 can fragmentize
> > memory rapidly.
> >
> > 2. when a special filesystem does not supply migratepage callback,
> > kernel would fallback to default function fallback_migrate_page.
> > but fallback_migrate_page could not migrate diry page nicely;
> > specially kcompactd with MIGRATE_SYNC_LIGHT could not migrate
> > diry pages due to this until clear_page_dirty_for_io in some
> > procedure. i think it is not suitable here in this scenario.
> > for dirty pages we should migrate it rather than skip or writeout
> > it in kcomapctd with MIGRATE_SYNC_LIGHT. i think this problem is
> > for all filesystem without migratepage not only for block device fs.
> >
> > So for compaction under large file writing supply migratepage for
> > def_blk_aops.
>
> Is this really safe to do? buffer_migrate_page() has some assumptions listed in
> its comment (and maybe more that are not listed). Do we know it's safe to use it
> for all def_blk_aops users?
So this is definitely *not* OK for ext4 which is using def_blk_aops. Ext4
will create temporary buffer heads that alias buffer heads attached to the
block device page when committing a transaction (see fs/jbd2/journal.c:
jbd2_journal_write_metadata_buffer()) and these will point to the original
page.
Now I'm not saying ext4 / jbd2 cannot be modified so that it somehow makes
it clear to the page migration code it should stay away from the page.
However it demostrates that this patch is not safe as is...
Honza
>
> > Link: http://lkml.kernel.org/r/1488937915-78955-1-git-send-email-zhouxianrong@huawei.com
> > Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: Mel Gorman <mgorman@techsingularity.net>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: <Mi.Sophia.Wang@huawei.com>
> > Cc: <zhouxiyu@huawei.com>
> > Cc: <weidu.du@huawei.com>
> > Cc: <zhangshiming5@huawei.com>
> > Cc: <won.ho.park@huawei.com>
> > Cc: <zhouxiaoyan1@huawei.com>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >
> > fs/block_dev.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff -puN fs/block_dev.c~compaction-add-def_blk_aops-migrate-function-for-memory-compaction fs/block_dev.c
> > --- a/fs/block_dev.c~compaction-add-def_blk_aops-migrate-function-for-memory-compaction
> > +++ a/fs/block_dev.c
> > @@ -2064,6 +2064,9 @@ static const struct address_space_operat
> > .releasepage = blkdev_releasepage,
> > .direct_IO = blkdev_direct_IO,
> > .is_dirty_writeback = buffer_check_dirty_writeback,
> > +#ifdef CONFIG_MIGRATION
> > + .migratepage = buffer_migrate_page,
> > +#endif
> > };
> >
> > #define BLKDEV_FALLOC_FL_SUPPORTED \
> > _
> >
> > Patches currently in -mm which might be from zhouxianrong@huawei.com are
> >
> > compaction-add-def_blk_aops-migrate-function-for-memory-compaction.patch
> >
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread