[RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Yang Shi]

Since tmpfs THP was supported in 4.8, hugetlbfs is not the only
filesystem with huge page support anymore. tmpfs can use huge page via
THP when mounting by "huge=" mount option.

When applications use huge page on hugetlbfs, it just need check the
filesystem magic number, but it is not enough for tmpfs. Make
stat.st_blksize return huge page size if it is mounted by appropriate
"huge=" option.

Some applications could benefit from this change, for example QEMU.
When use mmap file as guest VM backend memory, QEMU typically mmap the
file size plus one extra page. If the file is on hugetlbfs the extra
page is huge page size (i.e. 2MB), but it is still 4KB on tmpfs even
though THP is enabled. tmpfs THP requires VMA is huge page aligned, so
if 4KB page is used THP will not be used at all. The below /proc/meminfo
fragment shows the THP use of QEMU with 4K page:

ShmemHugePages:   679936 kB
ShmemPmdMapped:        0 kB

By reading st_blksize, tmpfs can use huge page, then /proc/meminfo looks
like:

ShmemHugePages:    77824 kB
ShmemPmdMapped:     6144 kB

statfs.f_bsize still returns 4KB for tmpfs since THP could be split, and it
also may fallback to 4KB page silently if there is not enough huge page.
Furthermore, different f_bsize makes max_blocks and free_blocks
calculation harder but without too much benefit. Returning huge page
size via stat.st_blksize sounds good enough.

Since PUD size huge page for THP has not been supported, now it just
returns HPAGE_PMD_SIZE.

Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Suggested-by: Christoph Hellwig <hch@infradead.org>
---
v2 --> v1:
* Adopted the suggestion from hch to return huge page size via st_blksize
  instead of creating a new flag.

 mm/shmem.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index b859192..3704258 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -39,6 +39,7 @@
 #include <asm/tlbflush.h> /* for arch/microblaze update_mmu_cache() */
 
 static struct vfsmount *shm_mnt;
+static bool is_huge = false;
 
 #ifdef CONFIG_SHMEM
 /*
@@ -995,6 +996,8 @@ static int shmem_getattr(const struct path *path, struct kstat *stat,
 		spin_unlock_irq(&info->lock);
 	}
 	generic_fillattr(inode, stat);
+	if (is_huge)
+		stat->blksize = HPAGE_PMD_SIZE;
 	return 0;
 }
 
@@ -3574,6 +3577,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
 					huge != SHMEM_HUGE_NEVER)
 				goto bad_val;
 			sbinfo->huge = huge;
+			is_huge = true;
 #endif
 #ifdef CONFIG_NUMA
 		} else if (!strcmp(this_char,"mpol")) {
-- 
1.8.3.1

Re: [RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Michal Hocko]

On Sat 21-04-18 00:33:59, Yang Shi wrote:
> Since tmpfs THP was supported in 4.8, hugetlbfs is not the only
> filesystem with huge page support anymore. tmpfs can use huge page via
> THP when mounting by "huge=" mount option.
> 
> When applications use huge page on hugetlbfs, it just need check the
> filesystem magic number, but it is not enough for tmpfs. Make
> stat.st_blksize return huge page size if it is mounted by appropriate
> "huge=" option.
> 
> Some applications could benefit from this change, for example QEMU.
> When use mmap file as guest VM backend memory, QEMU typically mmap the
> file size plus one extra page. If the file is on hugetlbfs the extra
> page is huge page size (i.e. 2MB), but it is still 4KB on tmpfs even
> though THP is enabled. tmpfs THP requires VMA is huge page aligned, so
> if 4KB page is used THP will not be used at all. The below /proc/meminfo
> fragment shows the THP use of QEMU with 4K page:
> 
> ShmemHugePages:   679936 kB
> ShmemPmdMapped:        0 kB
> 
> By reading st_blksize, tmpfs can use huge page, then /proc/meminfo looks
> like:
> 
> ShmemHugePages:    77824 kB
> ShmemPmdMapped:     6144 kB
> 
> statfs.f_bsize still returns 4KB for tmpfs since THP could be split, and it
> also may fallback to 4KB page silently if there is not enough huge page.
> Furthermore, different f_bsize makes max_blocks and free_blocks
> calculation harder but without too much benefit. Returning huge page
> size via stat.st_blksize sounds good enough.

I am not sure I understand the above. So does QEMU or other tmpfs users
rely on f_bsize to do mmap alignment tricks? Also I thought that THP
will be used on the first aligned address even when the initial/last
portion of the mapping is not THP aligned.

And more importantly
[...]
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -39,6 +39,7 @@
>  #include <asm/tlbflush.h> /* for arch/microblaze update_mmu_cache() */
>  
>  static struct vfsmount *shm_mnt;
> +static bool is_huge = false;
>  
>  #ifdef CONFIG_SHMEM
>  /*
> @@ -995,6 +996,8 @@ static int shmem_getattr(const struct path *path, struct kstat *stat,
>  		spin_unlock_irq(&info->lock);
>  	}
>  	generic_fillattr(inode, stat);
> +	if (is_huge)
> +		stat->blksize = HPAGE_PMD_SIZE;
>  	return 0;
>  }
>  
> @@ -3574,6 +3577,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>  					huge != SHMEM_HUGE_NEVER)
>  				goto bad_val;
>  			sbinfo->huge = huge;
> +			is_huge = true;

Huh! How come this is a global flag. What if we have multiple shmem
mounts some with huge pages enabled and some without? Btw. we seem to
already have that information stored in the supperblock
		} else if (!strcmp(this_char, "huge")) {
			int huge;
			huge = shmem_parse_huge(value);
			if (huge < 0)
				goto bad_val;
			if (!has_transparent_hugepage() &&
					huge != SHMEM_HUGE_NEVER)
				goto bad_val;
			sbinfo->huge = huge;
-- 
Michal Hocko
SUSE Labs

Re: [RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Yang Shi]

On 4/22/18 6:47 PM, Michal Hocko wrote:
> On Sat 21-04-18 00:33:59, Yang Shi wrote:
>> Since tmpfs THP was supported in 4.8, hugetlbfs is not the only
>> filesystem with huge page support anymore. tmpfs can use huge page via
>> THP when mounting by "huge=" mount option.
>>
>> When applications use huge page on hugetlbfs, it just need check the
>> filesystem magic number, but it is not enough for tmpfs. Make
>> stat.st_blksize return huge page size if it is mounted by appropriate
>> "huge=" option.
>>
>> Some applications could benefit from this change, for example QEMU.
>> When use mmap file as guest VM backend memory, QEMU typically mmap the
>> file size plus one extra page. If the file is on hugetlbfs the extra
>> page is huge page size (i.e. 2MB), but it is still 4KB on tmpfs even
>> though THP is enabled. tmpfs THP requires VMA is huge page aligned, so
>> if 4KB page is used THP will not be used at all. The below /proc/meminfo
>> fragment shows the THP use of QEMU with 4K page:
>>
>> ShmemHugePages:   679936 kB
>> ShmemPmdMapped:        0 kB
>>
>> By reading st_blksize, tmpfs can use huge page, then /proc/meminfo looks
>> like:
>>
>> ShmemHugePages:    77824 kB
>> ShmemPmdMapped:     6144 kB
>>
>> statfs.f_bsize still returns 4KB for tmpfs since THP could be split, and it
>> also may fallback to 4KB page silently if there is not enough huge page.
>> Furthermore, different f_bsize makes max_blocks and free_blocks
>> calculation harder but without too much benefit. Returning huge page
>> size via stat.st_blksize sounds good enough.
> I am not sure I understand the above. So does QEMU or other tmpfs users
> rely on f_bsize to do mmap alignment tricks? Also I thought that THP

QEMU doesn't. It just check filesystem magic number now, if it is 
hugetlbfs, then it do mmap on huge page size alignment.

> will be used on the first aligned address even when the initial/last
> portion of the mapping is not THP aligned.

No, my test shows it is not. And, transhuge_vma_suitable() does check 
the virtual address alignment. If it is not huge page size aligned, it 
will not set PMD for huge page.

>
> And more importantly
> [...]
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -39,6 +39,7 @@
>>   #include <asm/tlbflush.h> /* for arch/microblaze update_mmu_cache() */
>>   
>>   static struct vfsmount *shm_mnt;
>> +static bool is_huge = false;
>>   
>>   #ifdef CONFIG_SHMEM
>>   /*
>> @@ -995,6 +996,8 @@ static int shmem_getattr(const struct path *path, struct kstat *stat,
>>   		spin_unlock_irq(&info->lock);
>>   	}
>>   	generic_fillattr(inode, stat);
>> +	if (is_huge)
>> +		stat->blksize = HPAGE_PMD_SIZE;
>>   	return 0;
>>   }
>>   
>> @@ -3574,6 +3577,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>>   					huge != SHMEM_HUGE_NEVER)
>>   				goto bad_val;
>>   			sbinfo->huge = huge;
>> +			is_huge = true;
> Huh! How come this is a global flag. What if we have multiple shmem
> mounts some with huge pages enabled and some without? Btw. we seem to
> already have that information stored in the supperblock
> 		} else if (!strcmp(this_char, "huge")) {
> 			int huge;
> 			huge = shmem_parse_huge(value);
> 			if (huge < 0)
> 				goto bad_val;
> 			if (!has_transparent_hugepage() &&
> 					huge != SHMEM_HUGE_NEVER)
> 				goto bad_val;
> 			sbinfo->huge = huge;

Aha, my bad. I should used SHMEM_SB(inode->i_sb) to get shmem_sb_info 
then check the huge. Thanks a lot for catching this. Will fix in new 
version.

Yang

Re: [RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Michal Hocko]

On Sun 22-04-18 21:28:59, Yang Shi wrote:
> 
> 
> On 4/22/18 6:47 PM, Michal Hocko wrote:
[...]
> > will be used on the first aligned address even when the initial/last
> > portion of the mapping is not THP aligned.
> 
> No, my test shows it is not. And, transhuge_vma_suitable() does check the
> virtual address alignment. If it is not huge page size aligned, it will not
> set PMD for huge page.

It's been quite some time since I've looked at that code but I think you
are wrong. It just doesn't make sense to make the THP decision on the
VMA alignment much. Kirill, can you clarify please?

Please note that I have no objections to actually export the huge page
size as the max block size but your changelog just doesn't make any
sense to me.
-- 
Michal Hocko
SUSE Labs

Re: [RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Yang Shi]

On 4/23/18 9:04 AM, Michal Hocko wrote:
> On Sun 22-04-18 21:28:59, Yang Shi wrote:
>>
>> On 4/22/18 6:47 PM, Michal Hocko wrote:
> [...]
>>> will be used on the first aligned address even when the initial/last
>>> portion of the mapping is not THP aligned.
>> No, my test shows it is not. And, transhuge_vma_suitable() does check the
>> virtual address alignment. If it is not huge page size aligned, it will not
>> set PMD for huge page.
> It's been quite some time since I've looked at that code but I think you
> are wrong. It just doesn't make sense to make the THP decision on the
> VMA alignment much. Kirill, can you clarify please?

In the test, QEMU is trying to mmap a file (16GB in my configuration) + 
a guard page. If the page size is 4KB, there not any pages are mapped by 
PMD, but if the page size is 2MB (huge page aligned) we can see a lot 
pages are mapped by PMD. The test result is showed in the commit log.

So, if your assumption is right, there must be something wrong in THP code.

>
> Please note that I have no objections to actually export the huge page
> size as the max block size but your changelog just doesn't make any
> sense to me.

Thanks,
Yang

Re: [RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Yang Shi]

On 4/23/18 9:04 AM, Michal Hocko wrote:
> On Sun 22-04-18 21:28:59, Yang Shi wrote:
>>
>> On 4/22/18 6:47 PM, Michal Hocko wrote:
> [...]
>>> will be used on the first aligned address even when the initial/last
>>> portion of the mapping is not THP aligned.
>> No, my test shows it is not. And, transhuge_vma_suitable() does check the
>> virtual address alignment. If it is not huge page size aligned, it will not
>> set PMD for huge page.
> It's been quite some time since I've looked at that code but I think you
> are wrong. It just doesn't make sense to make the THP decision on the
> VMA alignment much. Kirill, can you clarify please?

Thanks a lot Michal and Kirill to elaborate how tmpfs THP make pmd map.

I did a quick test, THP will be PMD mapped as long as :
* hint address is huge page aligned if MAP_FIXED
Or
* offset is huge page aligned
And
* The size is big enough (>= huge page size)

This test does verify what Kirill said. And, I dig into a little further 
qemu code and did strace, qemu does try to mmap the file to non huge 
page aligned address with MAP_FIXED.

I will correct the commit log then submit v4.

Yang

>
> Please note that I have no objections to actually export the huge page
> size as the max block size but your changelog just doesn't make any
> sense to me.

Re: [RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Michal Hocko]

On Mon 23-04-18 21:41:50, Yang Shi wrote:
> 
> 
> On 4/23/18 9:04 AM, Michal Hocko wrote:
> > On Sun 22-04-18 21:28:59, Yang Shi wrote:
> > > 
> > > On 4/22/18 6:47 PM, Michal Hocko wrote:
> > [...]
> > > > will be used on the first aligned address even when the initial/last
> > > > portion of the mapping is not THP aligned.
> > > No, my test shows it is not. And, transhuge_vma_suitable() does check the
> > > virtual address alignment. If it is not huge page size aligned, it will not
> > > set PMD for huge page.
> > It's been quite some time since I've looked at that code but I think you
> > are wrong. It just doesn't make sense to make the THP decision on the
> > VMA alignment much. Kirill, can you clarify please?
> 
> Thanks a lot Michal and Kirill to elaborate how tmpfs THP make pmd map.
> 
> I did a quick test, THP will be PMD mapped as long as :
> * hint address is huge page aligned if MAP_FIXED
> Or
> * offset is huge page aligned
> And
> * The size is big enough (>= huge page size)
> 
> This test does verify what Kirill said. And, I dig into a little further
> qemu code and did strace, qemu does try to mmap the file to non huge page
> aligned address with MAP_FIXED.

Does it make sense to contact Qemu developers and probably fix this?

-- 
Michal Hocko
SUSE Labs

Re: [RFC v2 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Yang Shi]

On 4/24/18 6:43 AM, Michal Hocko wrote:
> On Mon 23-04-18 21:41:50, Yang Shi wrote:
>>
>> On 4/23/18 9:04 AM, Michal Hocko wrote:
>>> On Sun 22-04-18 21:28:59, Yang Shi wrote:
>>>> On 4/22/18 6:47 PM, Michal Hocko wrote:
>>> [...]
>>>>> will be used on the first aligned address even when the initial/last
>>>>> portion of the mapping is not THP aligned.
>>>> No, my test shows it is not. And, transhuge_vma_suitable() does check the
>>>> virtual address alignment. If it is not huge page size aligned, it will not
>>>> set PMD for huge page.
>>> It's been quite some time since I've looked at that code but I think you
>>> are wrong. It just doesn't make sense to make the THP decision on the
>>> VMA alignment much. Kirill, can you clarify please?
>> Thanks a lot Michal and Kirill to elaborate how tmpfs THP make pmd map.
>>
>> I did a quick test, THP will be PMD mapped as long as :
>> * hint address is huge page aligned if MAP_FIXED
>> Or
>> * offset is huge page aligned
>> And
>> * The size is big enough (>= huge page size)
>>
>> This test does verify what Kirill said. And, I dig into a little further
>> qemu code and did strace, qemu does try to mmap the file to non huge page
>> aligned address with MAP_FIXED.
> Does it make sense to contact Qemu developers and probably fix this?

Yes, I think so. We can submit a bug report to QEMU.

>