[PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Gioh Kim]

Hello,

This patch try to solve problem that a long-lasting page caches of
ext4 superblock and journaling of superblock disturb page migration.

I've been testing CMA feature on my ARM-based platform
and found that two page caches cannot be migrated.
They are page caches of superblock of ext4 filesystem and its journaling data.

Current ext4 reads superblock with sb_bread() that allocates page
from movable area. But the problem is that ext4 hold the page until
it is unmounted. If root filesystem is ext4 the page cannot be migrated
forever.
And also the journaling data for the superblock cannot be migreated.

I introduce new APIs that allocates page cache with specific flag passed by an
argument.
*_gfp APIs are for user want to set page allocation flag for page cache
allocation.
And *_unmovable APIs are for the user wants to allocate page cache from
non-movable area.

It is useful for ext4/ext3 and others that want to hold page cache for a long
time.

I have 3 patchs:

1. Patch 1/3: introduce a new API that create page cache with allocation flag
2. Patch 2/3: have ext4 use the new API to read superblock
3. Patch 3/3: have jbd/jbd2 use the new API to make journaling of superblock

This patchset is based on linux-next-20140814.

Thanks a lot.

Gioh Kim (3):
  fs.c: support buffer cache allocations with gfp modifiers
  ext4: use non-movable memory for the ext4 superblock
  jbd/jbd2: use non-movable memory for the jbd superblock

 fs/buffer.c                 |   45 ++++++++++++++++++++++++-----------------
 fs/ext4/super.c             |    6 +++---
 fs/jbd/journal.c            |    2 +-
 fs/jbd2/journal.c           |    2 +-
 include/linux/buffer_head.h |   47 ++++++++++++++++++++++++++++++++++++++-----
 5 files changed, 73 insertions(+), 29 deletions(-)

-- 
1.7.9.5

[PATCHv4 1/3] fs.c: support buffer cache allocations with gfp modifiers

[Gioh Kim]

A buffer cache is allocated from movable area
because it is referred for a while and released soon.
But some filesystems are taking buffer cache for a long time
and it can disturb page migration.

New APIs are introduced to allocate buffer cache
with user specific flag.
*_gfp APIs are for user want to set page allocation flag for page cache
allocation.
And *_unmovable APIs are for the user wants to allocate page cache from
non-movable area.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/buffer.c                 |   45 ++++++++++++++++++++++++-----------------
 include/linux/buffer_head.h |   47 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 68 insertions(+), 24 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 8f05111..9a6029e 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -993,7 +993,7 @@ init_page_buffers(struct page *page, struct block_device *bdev,
  */
 static int
 grow_dev_page(struct block_device *bdev, sector_t block,
-		pgoff_t index, int size, int sizebits)
+	      pgoff_t index, int size, int sizebits, gfp_t gfp)
 {
 	struct inode *inode = bdev->bd_inode;
 	struct page *page;
@@ -1002,8 +1002,8 @@ grow_dev_page(struct block_device *bdev, sector_t block,
 	int ret = 0;		/* Will call free_more_memory() */
 	gfp_t gfp_mask;
 
-	gfp_mask = mapping_gfp_mask(inode->i_mapping) & ~__GFP_FS;
-	gfp_mask |= __GFP_MOVABLE;
+	gfp_mask = (mapping_gfp_mask(inode->i_mapping) & ~__GFP_FS) | gfp;
+
 	/*
 	 * XXX: __getblk_slow() can not really deal with failure and
 	 * will endlessly loop on improvised global reclaim.  Prefer
@@ -1058,7 +1058,7 @@ failed:
  * that page was dirty, the buffers are set dirty also.
  */
 static int
-grow_buffers(struct block_device *bdev, sector_t block, int size)
+grow_buffers(struct block_device *bdev, sector_t block, int size, gfp_t gfp)
 {
 	pgoff_t index;
 	int sizebits;
@@ -1085,11 +1085,12 @@ grow_buffers(struct block_device *bdev, sector_t block, int size)
 	}
 
 	/* Create a page with the proper size buffers.. */
-	return grow_dev_page(bdev, block, index, size, sizebits);
+	return grow_dev_page(bdev, block, index, size, sizebits, gfp);
 }
 
-static struct buffer_head *
-__getblk_slow(struct block_device *bdev, sector_t block, int size)
+struct buffer_head *
+__getblk_slow(struct block_device *bdev, sector_t block,
+	     unsigned size, gfp_t gfp)
 {
 	/* Size must be multiple of hard sectorsize */
 	if (unlikely(size & (bdev_logical_block_size(bdev)-1) ||
@@ -1111,13 +1112,14 @@ __getblk_slow(struct block_device *bdev, sector_t block, int size)
 		if (bh)
 			return bh;
 
-		ret = grow_buffers(bdev, block, size);
+		ret = grow_buffers(bdev, block, size, gfp);
 		if (ret < 0)
 			return NULL;
 		if (ret == 0)
 			free_more_memory();
 	}
 }
+EXPORT_SYMBOL(__getblk_slow);
 
 /*
  * The relationship between dirty buffers and dirty pages:
@@ -1371,24 +1373,25 @@ __find_get_block(struct block_device *bdev, sector_t block, unsigned size)
 EXPORT_SYMBOL(__find_get_block);
 
 /*
- * __getblk will locate (and, if necessary, create) the buffer_head
+ * __getblk_gfp() will locate (and, if necessary, create) the buffer_head
  * which corresponds to the passed block_device, block and size. The
  * returned buffer has its reference count incremented.
  *
- * __getblk() will lock up the machine if grow_dev_page's try_to_free_buffers()
- * attempt is failing.  FIXME, perhaps?
+ * __getblk_gfp() will lock up the machine if grow_dev_page's
+ * try_to_free_buffers() attempt is failing.  FIXME, perhaps?
  */
 struct buffer_head *
-__getblk(struct block_device *bdev, sector_t block, unsigned size)
+__getblk_gfp(struct block_device *bdev, sector_t block,
+	     unsigned size, gfp_t gfp)
 {
 	struct buffer_head *bh = __find_get_block(bdev, block, size);
 
 	might_sleep();
 	if (bh == NULL)
-		bh = __getblk_slow(bdev, block, size);
+		bh = __getblk_slow(bdev, block, size, gfp);
 	return bh;
 }
-EXPORT_SYMBOL(__getblk);
+EXPORT_SYMBOL(__getblk_gfp);
 
 /*
  * Do async read-ahead on a buffer..
@@ -1404,24 +1407,28 @@ void __breadahead(struct block_device *bdev, sector_t block, unsigned size)
 EXPORT_SYMBOL(__breadahead);
 
 /**
- *  __bread() - reads a specified block and returns the bh
+ *  __bread_gfp() - reads a specified block and returns the bh
  *  @bdev: the block_device to read from
  *  @block: number of block
  *  @size: size (in bytes) to read
- * 
+ *  @gfp: page allocation flag
+ *
  *  Reads a specified block, and returns buffer head that contains it.
+ *  The page cache can be allocated from non-movable area
+ *  not to prevent page migration if you set gfp to zero.
  *  It returns NULL if the block was unreadable.
  */
 struct buffer_head *
-__bread(struct block_device *bdev, sector_t block, unsigned size)
+__bread_gfp(struct block_device *bdev, sector_t block,
+		   unsigned size, gfp_t gfp)
 {
-	struct buffer_head *bh = __getblk(bdev, block, size);
+	struct buffer_head *bh = __getblk_gfp(bdev, block, size, gfp);
 
 	if (likely(bh) && !buffer_uptodate(bh))
 		bh = __bread_slow(bh);
 	return bh;
 }
-EXPORT_SYMBOL(__bread);
+EXPORT_SYMBOL(__bread_gfp);
 
 /*
  * invalidate_bh_lrus() is called rarely - but not only at unmount.
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 324329c..73b4522 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -175,12 +175,13 @@ void __wait_on_buffer(struct buffer_head *);
 wait_queue_head_t *bh_waitq_head(struct buffer_head *bh);
 struct buffer_head *__find_get_block(struct block_device *bdev, sector_t block,
 			unsigned size);
-struct buffer_head *__getblk(struct block_device *bdev, sector_t block,
-			unsigned size);
+struct buffer_head *__getblk_gfp(struct block_device *bdev, sector_t block,
+				  unsigned size, gfp_t gfp);
 void __brelse(struct buffer_head *);
 void __bforget(struct buffer_head *);
 void __breadahead(struct block_device *, sector_t block, unsigned int size);
-struct buffer_head *__bread(struct block_device *, sector_t block, unsigned size);
+struct buffer_head *__bread_gfp(struct block_device *,
+				sector_t block, unsigned size, gfp_t gfp);
 void invalidate_bh_lrus(void);
 struct buffer_head *alloc_buffer_head(gfp_t gfp_flags);
 void free_buffer_head(struct buffer_head * bh);
@@ -295,7 +296,13 @@ static inline void bforget(struct buffer_head *bh)
 static inline struct buffer_head *
 sb_bread(struct super_block *sb, sector_t block)
 {
-	return __bread(sb->s_bdev, block, sb->s_blocksize);
+	return __bread_gfp(sb->s_bdev, block, sb->s_blocksize, __GFP_MOVABLE);
+}
+
+static inline struct buffer_head *
+sb_bread_unmovable(struct super_block *sb, sector_t block)
+{
+	return __bread_gfp(sb->s_bdev, block, sb->s_blocksize, 0);
 }
 
 static inline void
@@ -307,7 +314,7 @@ sb_breadahead(struct super_block *sb, sector_t block)
 static inline struct buffer_head *
 sb_getblk(struct super_block *sb, sector_t block)
 {
-	return __getblk(sb->s_bdev, block, sb->s_blocksize);
+	return __getblk_gfp(sb->s_bdev, block, sb->s_blocksize, __GFP_MOVABLE);
 }
 
 static inline struct buffer_head *
@@ -344,6 +351,36 @@ static inline void lock_buffer(struct buffer_head *bh)
 		__lock_buffer(bh);
 }
 
+static inline struct buffer_head *getblk_unmovable(struct block_device *bdev,
+						   sector_t block,
+						   unsigned size)
+{
+	return __getblk_gfp(bdev, block, size, 0);
+}
+
+static inline struct buffer_head *__getblk(struct block_device *bdev,
+					   sector_t block,
+					   unsigned size)
+{
+	return __getblk_gfp(bdev, block, size, __GFP_MOVABLE);
+}
+
+/**
+ *  __bread() - reads a specified block and returns the bh
+ *  @bdev: the block_device to read from
+ *  @block: number of block
+ *  @size: size (in bytes) to read
+ *
+ *  Reads a specified block, and returns buffer head that contains it.
+ *  The page cache is allocated from movable area so that it can be migrated.
+ *  It returns NULL if the block was unreadable.
+ */
+static inline struct buffer_head *
+__bread(struct block_device *bdev, sector_t block, unsigned size)
+{
+	return __bread_gfp(bdev, block, size, __GFP_MOVABLE);
+}
+
 extern int __set_page_dirty_buffers(struct page *page);
 
 #else /* CONFIG_BLOCK */
-- 
1.7.9.5

Re: [PATCHv4 1/3] fs.c: support buffer cache allocations with gfp modifiers

[Theodore Ts'o]

On Thu, Sep 04, 2014 at 04:29:39PM +0900, Gioh Kim wrote:
> A buffer cache is allocated from movable area
> because it is referred for a while and released soon.
> But some filesystems are taking buffer cache for a long time
> and it can disturb page migration.
> 
> New APIs are introduced to allocate buffer cache
> with user specific flag.
> *_gfp APIs are for user want to set page allocation flag for page cache
> allocation.
> And *_unmovable APIs are for the user wants to allocate page cache from
> non-movable area.
> 
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, I've applied this to the ext4 git tree with a slightly
rewritten commit description (for clarity).

						- Ted

[PATCHv4 2/3] ext4: use non-movable memory for the ext4 superblock

[Gioh Kim]

A buffer-cache for superblock is disturbing page migration,
because the buffer-cache is not released until unmount.
The buffer-cache must be allocated from non-movable area.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/super.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 32b43ad..e4b62b3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3435,7 +3435,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		logical_sb_block = sb_block;
 	}
 
-	if (!(bh = sb_bread(sb, logical_sb_block))) {
+	if (!(bh = sb_bread_unmovable(sb, logical_sb_block))) {
 		ext4_msg(sb, KERN_ERR, "unable to read superblock");
 		goto out_fail;
 	}
@@ -3645,7 +3645,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		brelse(bh);
 		logical_sb_block = sb_block * EXT4_MIN_BLOCK_SIZE;
 		offset = do_div(logical_sb_block, blocksize);
-		bh = sb_bread(sb, logical_sb_block);
+		bh = sb_bread_unmovable(sb, logical_sb_block);
 		if (!bh) {
 			ext4_msg(sb, KERN_ERR,
 			       "Can't read superblock on 2nd try");
@@ -3867,7 +3867,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 
 	for (i = 0; i < db_count; i++) {
 		block = descriptor_loc(sb, logical_sb_block, i);
-		sbi->s_group_desc[i] = sb_bread(sb, block);
+		sbi->s_group_desc[i] = sb_bread_unmovable(sb, block);
 		if (!sbi->s_group_desc[i]) {
 			ext4_msg(sb, KERN_ERR,
 			       "can't read group descriptor %d", i);
-- 
1.7.9.5

Re: [PATCHv4 2/3] ext4: use non-movable memory for the ext4 superblock

[Theodore Ts'o]

On Thu, Sep 04, 2014 at 04:29:40PM +0900, Gioh Kim wrote:
> A buffer-cache for superblock is disturbing page migration,
> because the buffer-cache is not released until unmount.
> The buffer-cache must be allocated from non-movable area.
> 
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, I've applied this to the ext4 git tree with a slightly
rewritten commit description (for clarity).

						- Ted

[PATCHv4 3/3] jbd/jbd2: use non-movable memory for the jbd superblock

[Gioh Kim]

A long-lasting buffer-cache can distrub page migration so that
it must be allocated from non-movable area.

The journal_init_inode is creating a buffer-cache for superblock journaling.
The superblock exists until system shutdown so that the buffer-cache
for the superblock would also exist for a long time
and it can distrub page migration.

This patch make the buffer-cache be allocated from non-movable area
not to distrub page migration.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/jbd/journal.c  |    2 +-
 fs/jbd2/journal.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index 06fe11e..aab8549 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -886,7 +886,7 @@ journal_t * journal_init_inode (struct inode *inode)
 		goto out_err;
 	}
 
-	bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
+	bh = getblk_unmovable(journal->j_dev, blocknr, journal->j_blocksize);
 	if (!bh) {
 		printk(KERN_ERR
 		       "%s: Cannot get buffer for journal superblock\n",
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 67b8e30..65bd3b1 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1237,7 +1237,7 @@ journal_t * jbd2_journal_init_inode (struct inode *inode)
 		goto out_err;
 	}
 
-	bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
+	bh = getblk_unmovable(journal->j_dev, blocknr, journal->j_blocksize);
 	if (!bh) {
 		printk(KERN_ERR
 		       "%s: Cannot get buffer for journal superblock\n",
-- 
1.7.9.5

Re: [PATCHv4 3/3] jbd/jbd2: use non-movable memory for the jbd superblock

[Theodore Ts'o]

On Thu, Sep 04, 2014 at 04:29:41PM +0900, Gioh Kim wrote:
> A long-lasting buffer-cache can distrub page migration so that
> it must be allocated from non-movable area.
> 
> The journal_init_inode is creating a buffer-cache for superblock journaling.
> The superblock exists until system shutdown so that the buffer-cache
> for the superblock would also exist for a long time
> and it can distrub page migration.
> 
> This patch make the buffer-cache be allocated from non-movable area
> not to distrub page migration.
> 
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, I've applied this to the ext4 git tree with a slightly
rewritten commit description (for clarity).

						- Ted

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Andrew Morton]

On Thu,  4 Sep 2014 16:29:38 +0900 Gioh Kim <gioh.kim@lge.com> wrote:

> This patch try to solve problem that a long-lasting page caches of
> ext4 superblock and journaling of superblock disturb page migration.
> 
> I've been testing CMA feature on my ARM-based platform
> and found that two page caches cannot be migrated.
> They are page caches of superblock of ext4 filesystem and its journaling data.
> 
> Current ext4 reads superblock with sb_bread() that allocates page
> from movable area. But the problem is that ext4 hold the page until
> it is unmounted. If root filesystem is ext4 the page cannot be migrated
> forever.
> And also the journaling data for the superblock cannot be migreated.
> 
> I introduce new APIs that allocates page cache with specific flag passed by an
> argument.
> *_gfp APIs are for user want to set page allocation flag for page cache
> allocation.
> And *_unmovable APIs are for the user wants to allocate page cache from
> non-movable area.
> 
> It is useful for ext4/ext3 and others that want to hold page cache for a long
> time.

Could we please have some detailed information about the real-world
effect of this patchset?

You earlier said "My test platform is currently selling item in the
market.  And also I test my patch when my platform is working as if
real user uses it.".

But what were the problems which were observed in standard kernels and
what effect did this patchset have upon them?  Some quantitative
measurements will really help here.

I'm trying to get an understanding of how effective and important the
change is, whether others will see similar benefits.  I'd also like to
understand how *complete* the fix is - were the problems which you
observed completely fixed, or do outstanding problems remain?

Thanks.

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Gioh Kim]

2014-09-05 오전 7:16, Andrew Morton 쓴 글:
> On Thu,  4 Sep 2014 16:29:38 +0900 Gioh Kim <gioh.kim@lge.com> wrote:
>
>> This patch try to solve problem that a long-lasting page caches of
>> ext4 superblock and journaling of superblock disturb page migration.
>>
>> I've been testing CMA feature on my ARM-based platform
>> and found that two page caches cannot be migrated.
>> They are page caches of superblock of ext4 filesystem and its journaling data.
>>
>> Current ext4 reads superblock with sb_bread() that allocates page
>> from movable area. But the problem is that ext4 hold the page until
>> it is unmounted. If root filesystem is ext4 the page cannot be migrated
>> forever.
>> And also the journaling data for the superblock cannot be migreated.
>>
>> I introduce new APIs that allocates page cache with specific flag passed by an
>> argument.
>> *_gfp APIs are for user want to set page allocation flag for page cache
>> allocation.
>> And *_unmovable APIs are for the user wants to allocate page cache from
>> non-movable area.
>>
>> It is useful for ext4/ext3 and others that want to hold page cache for a long
>> time.
>
> Could we please have some detailed information about the real-world
> effect of this patchset?
>
> You earlier said "My test platform is currently selling item in the
> market.  And also I test my patch when my platform is working as if
> real user uses it.".

OK. I'm writing details as possible as I can.
Please feel free to request me more information.

My platform is TV and 1GB system memory and 256MB CMA memory.
I want to use full 256MB CMA memory.

>
> But what were the problems which were observed in standard kernels and
> what effect did this patchset have upon them?  Some quantitative
> measurements will really help here.

The problem is that I cannot allocate entire CMA memory.
Actually the problem is not found without Joonsoo's patch: https://lkml.org/lkml/2014/5/28/64.
Without it CMA memory is free and every CMA-memory allocation is successed.

If the Joonsoo's patch is applied, the CMA memory is allocated generally when system boots-up.
Therefore superblocks of mounted filesystem and buffer cache of it are allocation from CMA memory.

I have three ext4 partitions and one squash partition.
The squash filesystem has no problem. It holds buffer-cache temporarily.

But each ext4 partition holds 2 buffer-cache until unmounted (one for sb and one for journal)
so that I found 2 or 3 pages, the page are storing buffer-cache, are busy in my platform
when I try to allocate 256MB, entiry CMA memory.
So my allocation fails.

This patchset makes the long-lasting buffer-caches be allocated in non-CMA area.
Therefore I can success to allocate the entire CMA memory always.

Please tell me what I have to measure quantitatively?
What I know is every ext4 filesystem has 2 long-lasting buffer-cache
that are released when it is unmounted.

I applied this patch and try to allocate the entire CMA memory almost 100-times.
And I successed always.

>
> I'm trying to get an understanding of how effective and important the
> change is, whether others will see similar benefits.  I'd also like to
> understand how *complete* the fix is - were the problems which you
> observed completely fixed, or do outstanding problems remain?

I think this patch has benefits only for systems that use CMA or HOTPLUG feature.
As I mentioned above, the problem is not occured without Joonsoo's patch that allocates CMA area frequently.

If a system want to use CMA/HOTPLUG feature, I think, this patch is very important.
The problem is only several pages but several MB can be wasted considering an align of allocation size.
If allocation size align is 16MB and one page is busy, 16MB can be wasted.
For embedded system like TV 16MB is really big issue.

I beleive the problem is completely fixed with my patch.
I've tested many times for several days and reviewed ext4 code that deals with buffer-header.
I couldn't find any other problem.

I'm sorry to confuse you with my poor English.
Please reply me whatever you need.

Next week is Korean thanksgiving holidays.
I think I can reply on Fri.

>
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Theodore Ts'o]

On Fri, Sep 05, 2014 at 09:37:05AM +0900, Gioh Kim wrote:
> >But what were the problems which were observed in standard kernels and
> >what effect did this patchset have upon them?  Some quantitative
> >measurements will really help here.
> 
> The problem is that I cannot allocate entire CMA memory.
> >
> Actually the problem is not found without Joonsoo's patch:
> https://lkml.org/lkml/2014/5/28/64.  Without it CMA memory is free
> and every CMA-memory allocation is successed.
> 
> If the Joonsoo's patch is applied, the CMA memory is allocated
> generally when system boots-up.

As I said earlier, I'm happy to carry this patch in the ext4 tree,
because as it turns out I could use this facility for another purpose
(to cause a few buffer cache allocations to happen with __GFP_NOFAIL).

I do have one question; I note that Joonsoo's patch dates back to May,
and yet this has not hit the mainline kernel, and I haven't seen any
discussions about this patch after May.  Has there been some pushback
from the mm maintainers about Joonsoo's approach with respect to this
patch?   What is the current status of that patch set?

Thanks,

					- Ted

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Joonsoo Kim]

On Thu, Sep 04, 2014 at 09:14:19PM -0400, Theodore Ts'o wrote:
> On Fri, Sep 05, 2014 at 09:37:05AM +0900, Gioh Kim wrote:
> > >But what were the problems which were observed in standard kernels and
> > >what effect did this patchset have upon them?  Some quantitative
> > >measurements will really help here.
> > 
> > The problem is that I cannot allocate entire CMA memory.
> > >
> > Actually the problem is not found without Joonsoo's patch:
> > https://lkml.org/lkml/2014/5/28/64.  Without it CMA memory is free
> > and every CMA-memory allocation is successed.
> > 
> > If the Joonsoo's patch is applied, the CMA memory is allocated
> > generally when system boots-up.
> 
> As I said earlier, I'm happy to carry this patch in the ext4 tree,
> because as it turns out I could use this facility for another purpose
> (to cause a few buffer cache allocations to happen with __GFP_NOFAIL).
> 
> I do have one question; I note that Joonsoo's patch dates back to May,
> and yet this has not hit the mainline kernel, and I haven't seen any
> discussions about this patch after May.  Has there been some pushback
> from the mm maintainers about Joonsoo's approach with respect to this
> patch?   What is the current status of that patch set?

Hello,

That patchset is postponed, but will be continued. The reason is that
another bugs turn up frequently if that patchset is applied. I will
fix this bug first and will re-submit that patchset. Following is
the attempt to fix this bug.

https://lkml.org/lkml/2014/8/26/147

Thanks.

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Theodore Ts'o]

Joonson,

Thanks for the update.  I've applied Gioh's patches to the ext4 tree,
but I'd appreciate a further clarification.  My understanding with the
problem you were trying to address is that with the current CMA
implementation, kswapd was getting activiated too early, yes?

But it would still be a good idea to try to use non-moveable memory in
preference in favor of CMA memory; even if the page migration can move
the contents of the page elsewhere, wouldn't be better to avoid
needing to do the page migation in the first place.  Given that the
ext4 file systems are getting mounted very early in the boot process,
when there should be plenty of CMA and non-CMA elegible memory
available, why was CMA memory getting selected for the buffer cache
allocations when non-CMA memory available?

In other words, even without Gioh's patch to force the use of non-CMA
eligible memory, wouldn't it be better if the memory allocator used
non-CMA preferentially if it were available.  This should be
orthogonal to whether or not kswaped gets activiated, right?

Regards,

							- Ted

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Joonsoo Kim]

On Thu, Sep 04, 2014 at 11:17:35PM -0400, Theodore Ts'o wrote:
> Joonson,
> 
> Thanks for the update.  I've applied Gioh's patches to the ext4 tree,
> but I'd appreciate a further clarification.  My understanding with the
> problem you were trying to address is that with the current CMA
> implementation, kswapd was getting activiated too early, yes?
> 
> But it would still be a good idea to try to use non-moveable memory in
> preference in favor of CMA memory; even if the page migration can move
> the contents of the page elsewhere, wouldn't be better to avoid
> needing to do the page migation in the first place.  Given that the
> ext4 file systems are getting mounted very early in the boot process,
> when there should be plenty of CMA and non-CMA elegible memory
> available, why was CMA memory getting selected for the buffer cache
> allocations when non-CMA memory available?
> 
> In other words, even without Gioh's patch to force the use of non-CMA
> eligible memory, wouldn't it be better if the memory allocator used
> non-CMA preferentially if it were available.  This should be
> orthogonal to whether or not kswaped gets activiated, right?

Hello,

Yes, similar with your understanding.

With current CMA implementation, page allocator doesn't returns
freepages in CMA reserved region until there is no freepage of movable
type. If there is no freepage of movable type, freepages in CMA reserved
region is allocated via fallback allocation. This approach is simliar
with what you suggested.

In that situation, kswapd would start to reclaim because free memory
is already too low. By this reclaim, freepages of movable type are
refilled and page allocator would stop to allocate freepages in CMA
reserved region. So, with above approach, system would work as if
it has only (total memory - CMA reserved memory) memory.

CMA is intended for using reserved memory on general purpose when
reserved memory isn't used. But current implementation don't do that.
My patch fixes this situation by allocating freepages in CMA reserved
region aggrressively.

I also test another approach, such as allocate freepage in CMA
reserved region as late as possible, which is also similar to your
suggestion and this doesn't works well. When reclaim is started,
too many pages reclaim at once, because lru list has successive pages
in CMA region and these doesn't help kswapd's reclaim. kswapd stop
reclaiming when freepage count is recovered. But, CMA pages isn't
counted for freepage for kswapd because they can't be usable for
unmovable, reclaimable allocation. So kswap reclaim too many pages
at once unnecessarilly.

More detailed explanation may be in following link.
https://lkml.org/lkml/2014/5/28/57

Thanks.

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Theodore Ts'o]

On Fri, Sep 05, 2014 at 04:32:48PM +0900, Joonsoo Kim wrote:
> I also test another approach, such as allocate freepage in CMA
> reserved region as late as possible, which is also similar to your
> suggestion and this doesn't works well. When reclaim is started,
> too many pages reclaim at once, because lru list has successive pages
> in CMA region and these doesn't help kswapd's reclaim. kswapd stop
> reclaiming when freepage count is recovered. But, CMA pages isn't
> counted for freepage for kswapd because they can't be usable for
> unmovable, reclaimable allocation. So kswap reclaim too many pages
> at once unnecessarilly.

Have you considered putting the pages in a CMA region in a separate
zone?  After all, that's what we originally did with brain-damaged
hardware that could only DMA into the low 16M of memory.  We just
reserved a separate zone for that?  That way, we could do
zone-directed reclaim and free pages in that zone, if that was what
was actually needed.

But if we would also preferentially avoid using pages from that zone
unless there was no choice, in order to avoid needing to do that
zone-directed reclaim.  Perhaps a similar solution could do done here?

	      			  	  - Ted

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Joonsoo Kim]

On Fri, Sep 05, 2014 at 10:14:16AM -0400, Theodore Ts'o wrote:
> On Fri, Sep 05, 2014 at 04:32:48PM +0900, Joonsoo Kim wrote:
> > I also test another approach, such as allocate freepage in CMA
> > reserved region as late as possible, which is also similar to your
> > suggestion and this doesn't works well. When reclaim is started,
> > too many pages reclaim at once, because lru list has successive pages
> > in CMA region and these doesn't help kswapd's reclaim. kswapd stop
> > reclaiming when freepage count is recovered. But, CMA pages isn't
> > counted for freepage for kswapd because they can't be usable for
> > unmovable, reclaimable allocation. So kswap reclaim too many pages
> > at once unnecessarilly.
> 
> Have you considered putting the pages in a CMA region in a separate
> zone?  After all, that's what we originally did with brain-damaged
> hardware that could only DMA into the low 16M of memory.  We just
> reserved a separate zone for that?  That way, we could do
> zone-directed reclaim and free pages in that zone, if that was what
> was actually needed.

Sorry for long delay. It was long holidays.

No, I haven't consider it. It sounds good idea to place the pages in a
CMA region into a separate zone. Perhaps we can remove one of
migratetype, MIGRATE_CMA, with this way and it would be a good long-term
architecture for CMA.

I don't know exact history and reason why CMA is implemented in current
form. Ccing some experts in this area.

Thanks.

Re: [PATCHv4 0/3] new APIs to allocate buffer-cache with user specific flag

[Minchan Kim]

On Mon, Sep 15, 2014 at 10:10:18AM +0900, Joonsoo Kim wrote:
> On Fri, Sep 05, 2014 at 10:14:16AM -0400, Theodore Ts'o wrote:
> > On Fri, Sep 05, 2014 at 04:32:48PM +0900, Joonsoo Kim wrote:
> > > I also test another approach, such as allocate freepage in CMA
> > > reserved region as late as possible, which is also similar to your
> > > suggestion and this doesn't works well. When reclaim is started,
> > > too many pages reclaim at once, because lru list has successive pages
> > > in CMA region and these doesn't help kswapd's reclaim. kswapd stop
> > > reclaiming when freepage count is recovered. But, CMA pages isn't
> > > counted for freepage for kswapd because they can't be usable for
> > > unmovable, reclaimable allocation. So kswap reclaim too many pages
> > > at once unnecessarilly.
> > 
> > Have you considered putting the pages in a CMA region in a separate
> > zone?  After all, that's what we originally did with brain-damaged
> > hardware that could only DMA into the low 16M of memory.  We just
> > reserved a separate zone for that?  That way, we could do
> > zone-directed reclaim and free pages in that zone, if that was what
> > was actually needed.
> 
> Sorry for long delay. It was long holidays.
> 
> No, I haven't consider it. It sounds good idea to place the pages in a
> CMA region into a separate zone. Perhaps we can remove one of
> migratetype, MIGRATE_CMA, with this way and it would be a good long-term
> architecture for CMA.

IIRC, Mel suggested two options, ZONE_MOVABLE zone and MIGRATE_ISOLATE.
Absolutely, movable zone option is better solution if we consider
interacting with reclaim but one problem was CMA had a specific
requirement for memory in the middle of an existing zone.
And his concern comes true.
Look at https://lkml.org/lkml/2014/5/28/64.
It starts to add more stuff in allocator's fast path to overcome the
problem. :(

Let's rethink. We already have a logic to handle overlapping nodes/zones
in compaction.c so isn't it possible to have discrete address ranges
in a movable zone? If so, movable zone can include specific ranges horrible
devices want and it could make allocation/reclaim logic simple than now and
add overheads to slow path(ie, linear pfn scanning logic of zone like
compaction).

> 
> I don't know exact history and reason why CMA is implemented in current
> form. Ccing some experts in this area.
> 
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Kind regards,
Minchan Kim