Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1696944 > unrolled thread

Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding definitions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding  definitions Michal Hocko <mhocko@kernel.org> - 2017-07-26 12:00 +0200
    Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding  definitions Michal Hocko <mhocko@kernel.org> - 2017-07-26 12:10 +0200
      Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding  definitions Mike Kravetz <mike.kravetz@oracle.com> - 2017-07-26 19:50 +0200
        Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding  definitions Matthew Wilcox <willy@infradead.org> - 2017-07-26 20:50 +0200
        Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding  definitions Michal Hocko <mhocko@kernel.org> - 2017-07-27 10:00 +0200
          Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding  definitions Mike Kravetz <mike.kravetz@oracle.com> - 2017-07-27 23:20 +0200
            Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding  definitions Michal Hocko <mhocko@kernel.org> - 2017-07-28 08:40 +0200

#1696944 — Re: [RFC PATCH 3/3] mm: shm: Use new hugetlb size encoding definitions

On Mon 17-07-17 15:28:01, Mike Kravetz wrote:
> Use the common definitions from hugetlb_encode.h header file for
> encoding hugetlb size definitions in shmget system call flags.  In
> addition, move these definitions to the from the internal to user
> (uapi) header file.

s@to the from@from@

> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>

with s@HUGETLB_FLAG_ENCODE__16GB@HUGETLB_FLAG_ENCODE_16GB@

Acked-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/shm.h      | 17 -----------------
>  include/uapi/linux/shm.h | 23 +++++++++++++++++++++--
>  2 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/shm.h b/include/linux/shm.h
> index 04e8818..d56285a 100644
> --- a/include/linux/shm.h
> +++ b/include/linux/shm.h
> @@ -27,23 +27,6 @@ struct shmid_kernel /* private to the kernel */
>  /* shm_mode upper byte flags */
>  #define	SHM_DEST	01000	/* segment will be destroyed on last detach */
>  #define SHM_LOCKED      02000   /* segment will not be swapped */
> -#define SHM_HUGETLB     04000   /* segment will use huge TLB pages */
> -#define SHM_NORESERVE   010000  /* don't check for reservations */
> -
> -/* Bits [26:31] are reserved */
> -
> -/*
> - * When SHM_HUGETLB is set bits [26:31] encode the log2 of the huge page size.
> - * This gives us 6 bits, which is enough until someone invents 128 bit address
> - * spaces.
> - *
> - * Assume these are all power of twos.
> - * When 0 use the default page size.
> - */
> -#define SHM_HUGE_SHIFT  26
> -#define SHM_HUGE_MASK   0x3f
> -#define SHM_HUGE_2MB    (21 << SHM_HUGE_SHIFT)
> -#define SHM_HUGE_1GB    (30 << SHM_HUGE_SHIFT)
>  
>  #ifdef CONFIG_SYSVIPC
>  struct sysv_shm {
> diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
> index 1fbf24e..329bc17 100644
> --- a/include/uapi/linux/shm.h
> +++ b/include/uapi/linux/shm.h
> @@ -3,6 +3,7 @@
>  
>  #include <linux/ipc.h>
>  #include <linux/errno.h>
> +#include <asm-generic/hugetlb_encode.h>
>  #ifndef __KERNEL__
>  #include <unistd.h>
>  #endif
> @@ -40,11 +41,29 @@ struct shmid_ds {
>  /* Include the definition of shmid64_ds and shminfo64 */
>  #include <asm/shmbuf.h>
>  
> -/* permission flag for shmget */
> +/* shmget() shmflg values. */
> +/* The bottom nine bits are the same as open(2) mode flags */
>  #define SHM_R		0400	/* or S_IRUGO from <linux/stat.h> */
>  #define SHM_W		0200	/* or S_IWUGO from <linux/stat.h> */
> +/* Bits 9 & 10 are IPC_CREAT and IPC_EXCL */
> +#define SHM_HUGETLB	04000	/* segment will use huge TLB pages */
> +#define	SHM_NORESERVE	010000	/* don't check for reservations */
>  
> -/* mode for attach */
> +/*
> + * Huge page size encoding when SHM_HUGETLB is specified, and a huge page
> + * size other than the default is desired.  See hugetlb_encode.h
> + */
> +#define SHM_HUGE_SHIFT	HUGETLB_FLAG_ENCODE_SHIFT
> +#define SHM_HUGE_MASK	HUGETLB_FLAG_ENCODE_MASK
> +#define MAP_HUGE_512KB	HUGETLB_FLAG_ENCODE_512KB
> +#define MAP_HUGE_1MB	HUGETLB_FLAG_ENCODE_1MB
> +#define MAP_HUGE_2MB	HUGETLB_FLAG_ENCODE_2MB
> +#define MAP_HUGE_8MB	HUGETLB_FLAG_ENCODE_8MB
> +#define MAP_HUGE_16MB	HUGETLB_FLAG_ENCODE_16MB
> +#define MAP_HUGE_1GB	HUGETLB_FLAG_ENCODE_1GB
> +#define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE__16GB
> +
> +/* shmat() shmflg values */
>  #define	SHM_RDONLY	010000	/* read-only access */
>  #define	SHM_RND		020000	/* round attach address to SHMLBA boundary */
>  #define	SHM_REMAP	040000	/* take-over region on attach */
> -- 
> 2.7.5
> 

-- 
Michal Hocko
SUSE Labs

[toc] | [next] | [standalone]

#1696954

On Wed 26-07-17 11:53:38, Michal Hocko wrote:
> On Mon 17-07-17 15:28:01, Mike Kravetz wrote:
> > Use the common definitions from hugetlb_encode.h header file for
> > encoding hugetlb size definitions in shmget system call flags.  In
> > addition, move these definitions to the from the internal to user
> > (uapi) header file.
> 
> s@to the from@from@
> 
> > 
> > Suggested-by: Matthew Wilcox <willy@infradead.org>
> > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> 
> with s@HUGETLB_FLAG_ENCODE__16GB@HUGETLB_FLAG_ENCODE_16GB@
> 
> Acked-by: Michal Hocko <mhocko@suse.com>

Btw. man page mentions only 2MB and 1GB, we should document others and
note that each arch might support only subset of them

> > +#define MAP_HUGE_512KB	HUGETLB_FLAG_ENCODE_512KB
> > +#define MAP_HUGE_1MB	HUGETLB_FLAG_ENCODE_1MB
> > +#define MAP_HUGE_2MB	HUGETLB_FLAG_ENCODE_2MB
> > +#define MAP_HUGE_8MB	HUGETLB_FLAG_ENCODE_8MB
> > +#define MAP_HUGE_16MB	HUGETLB_FLAG_ENCODE_16MB
> > +#define MAP_HUGE_1GB	HUGETLB_FLAG_ENCODE_1GB
> > +#define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE__16GB
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1697445

On 07/26/2017 03:07 AM, Michal Hocko wrote:
> On Wed 26-07-17 11:53:38, Michal Hocko wrote:
>> On Mon 17-07-17 15:28:01, Mike Kravetz wrote:
>>> Use the common definitions from hugetlb_encode.h header file for
>>> encoding hugetlb size definitions in shmget system call flags.  In
>>> addition, move these definitions to the from the internal to user
>>> (uapi) header file.
>>
>> s@to the from@from@
>>
>>>
>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>>
>> with s@HUGETLB_FLAG_ENCODE__16GB@HUGETLB_FLAG_ENCODE_16GB@
>>
>> Acked-by: Michal Hocko <mhocko@suse.com>
> 
> Btw. man page mentions only 2MB and 1GB, we should document others and
> note that each arch might support only subset of them

Thanks for looking at these Michal.
BTW, those definitions below are wrong.  They should be SHM_HUGE_*. :(

In the overview of this RFC, I mentioned still needing to address the
comment from Aneesh about splitting SHM_HUGE_* definitions into arch
specific header files.  This is how it is done for mmap.  If an arch
supports multiple huge page sizes, the 'asm/mman.h' contains definitions
for those sizes.  There will be a bit of churn (such as header file
renaming) to do this for shm as well.  So, I keep going back and forth
asking myself 'is it worth it'?  Some things to consider.

- We should be consistent between mmap and shm.  Also remember, that I
  will propose adding the same type of encoding to memfd_create.  So,
  three system calls will use the encoding.  They should be consistent.
- Adding the arch specific definitions seems the 'most correct', as a
  user can not use a definition not supported by the arch.  However,
  even if an arch supports a huge page size it does not mean that the
  running kernel supports that size.  Therefore, the folllowing is in
  the man page.
  "The  range  of  huge page sizes that are supported by the system
   can be discovered by listing  the  subdirectories  in
   /sys/kernel/mm/hugepages."
- Another alternative is to make all known huge page sizes available
  to all users.  This is 'easier' as the definitions can likely reside
  in a common header file.  The user will  need to determine what
  huge page sizes are supported by the running kernel as mentioned in
  the man page.

Any thoughts/suggestions on these alternatives?  I'll send out another
patch set based on comments.  In any case, I think mmap and shm need to
be the same.
-- 
Mike Kravetz

>>> +#define MAP_HUGE_512KB	HUGETLB_FLAG_ENCODE_512KB
>>> +#define MAP_HUGE_1MB	HUGETLB_FLAG_ENCODE_1MB
>>> +#define MAP_HUGE_2MB	HUGETLB_FLAG_ENCODE_2MB
>>> +#define MAP_HUGE_8MB	HUGETLB_FLAG_ENCODE_8MB
>>> +#define MAP_HUGE_16MB	HUGETLB_FLAG_ENCODE_16MB
>>> +#define MAP_HUGE_1GB	HUGETLB_FLAG_ENCODE_1GB
>>> +#define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE__16GB

[toc] | [prev] | [next] | [standalone]

#1697479

On Wed, Jul 26, 2017 at 10:39:30AM -0700, Mike Kravetz wrote:
> In the overview of this RFC, I mentioned still needing to address the
> comment from Aneesh about splitting SHM_HUGE_* definitions into arch
> specific header files.  This is how it is done for mmap.  If an arch
> supports multiple huge page sizes, the 'asm/mman.h' contains definitions
> for those sizes.  There will be a bit of churn (such as header file
> renaming) to do this for shm as well.  So, I keep going back and forth
> asking myself 'is it worth it'?  Some things to consider.
> 
> - We should be consistent between mmap and shm.  Also remember, that I
>   will propose adding the same type of encoding to memfd_create.  So,
>   three system calls will use the encoding.  They should be consistent.

I think mmap is wrong here.  User programs are generally not architecture
specific, so they'll have to test with ifdefs or something awful.
For all we know, POWER 14 and whatever x86 CPU comes out in 2030 will
support (nearly) arbitrary page sizes like Itanium does, and a user
program compiled today should be able to take advantage of it.

> - Adding the arch specific definitions seems the 'most correct', as a
>   user can not use a definition not supported by the arch.  However,
>   even if an arch supports a huge page size it does not mean that the
>   running kernel supports that size.  Therefore, the folllowing is in
>   the man page.
>   "The  range  of  huge page sizes that are supported by the system
>    can be discovered by listing  the  subdirectories  in
>    /sys/kernel/mm/hugepages."
> - Another alternative is to make all known huge page sizes available
>   to all users.  This is 'easier' as the definitions can likely reside
>   in a common header file.  The user will  need to determine what
>   huge page sizes are supported by the running kernel as mentioned in
>   the man page.

That's my preference.

[toc] | [prev] | [next] | [standalone]

#1697785

On Wed 26-07-17 10:39:30, Mike Kravetz wrote:
> On 07/26/2017 03:07 AM, Michal Hocko wrote:
> > On Wed 26-07-17 11:53:38, Michal Hocko wrote:
> >> On Mon 17-07-17 15:28:01, Mike Kravetz wrote:
> >>> Use the common definitions from hugetlb_encode.h header file for
> >>> encoding hugetlb size definitions in shmget system call flags.  In
> >>> addition, move these definitions to the from the internal to user
> >>> (uapi) header file.
> >>
> >> s@to the from@from@
> >>
> >>>
> >>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> >>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> >>
> >> with s@HUGETLB_FLAG_ENCODE__16GB@HUGETLB_FLAG_ENCODE_16GB@
> >>
> >> Acked-by: Michal Hocko <mhocko@suse.com>
> > 
> > Btw. man page mentions only 2MB and 1GB, we should document others and
> > note that each arch might support only subset of them
> 
> Thanks for looking at these Michal.
> BTW, those definitions below are wrong.  They should be SHM_HUGE_*. :(

Ups, and I completely missed that.

> In the overview of this RFC, I mentioned still needing to address the
> comment from Aneesh about splitting SHM_HUGE_* definitions into arch
> specific header files.  This is how it is done for mmap.  If an arch
> supports multiple huge page sizes, the 'asm/mman.h' contains definitions
> for those sizes.  There will be a bit of churn (such as header file
> renaming) to do this for shm as well.  So, I keep going back and forth
> asking myself 'is it worth it'?

Why cannot we use a generic header? Btw. I think it would be better for
MMAP definitions as well.

> Some things to consider.
> 
> - We should be consistent between mmap and shm.  Also remember, that I
>   will propose adding the same type of encoding to memfd_create.  So,
>   three system calls will use the encoding.  They should be consistent.

agreed

> - Adding the arch specific definitions seems the 'most correct', as a
>   user can not use a definition not supported by the arch.  However,
>   even if an arch supports a huge page size it does not mean that the
>   running kernel supports that size.  Therefore, the folllowing is in
>   the man page.
>   "The  range  of  huge page sizes that are supported by the system
>    can be discovered by listing  the  subdirectories  in
>    /sys/kernel/mm/hugepages."

Doesn't the respective call return -EINVAL on the unsupported hugepage
size?

> - Another alternative is to make all known huge page sizes available
>   to all users.  This is 'easier' as the definitions can likely reside
>   in a common header file.  The user will  need to determine what
>   huge page sizes are supported by the running kernel as mentioned in
>   the man page.

yes I think this makes more sense.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1698330

On 07/27/2017 12:50 AM, Michal Hocko wrote:
> On Wed 26-07-17 10:39:30, Mike Kravetz wrote:
>> On 07/26/2017 03:07 AM, Michal Hocko wrote:
>>> On Wed 26-07-17 11:53:38, Michal Hocko wrote:
>>>> On Mon 17-07-17 15:28:01, Mike Kravetz wrote:
>>>>> Use the common definitions from hugetlb_encode.h header file for
>>>>> encoding hugetlb size definitions in shmget system call flags.  In
>>>>> addition, move these definitions to the from the internal to user
>>>>> (uapi) header file.
>>>>
>>>> s@to the from@from@
>>>>
>>>>>
>>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>>>>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>>>>
>>>> with s@HUGETLB_FLAG_ENCODE__16GB@HUGETLB_FLAG_ENCODE_16GB@
>>>>
>>>> Acked-by: Michal Hocko <mhocko@suse.com>
>>>
>>> Btw. man page mentions only 2MB and 1GB, we should document others and
>>> note that each arch might support only subset of them
>>
>> Thanks for looking at these Michal.
>> BTW, those definitions below are wrong.  They should be SHM_HUGE_*. :(
> 
> Ups, and I completely missed that.
> 
>> In the overview of this RFC, I mentioned still needing to address the
>> comment from Aneesh about splitting SHM_HUGE_* definitions into arch
>> specific header files.  This is how it is done for mmap.  If an arch
>> supports multiple huge page sizes, the 'asm/mman.h' contains definitions
>> for those sizes.  There will be a bit of churn (such as header file
>> renaming) to do this for shm as well.  So, I keep going back and forth
>> asking myself 'is it worth it'?
> 
> Why cannot we use a generic header? Btw. I think it would be better for
> MMAP definitions as well.

I assume you are asking about a uapi asm-generic header file?  Currently
mmap has two such files:  mman.h and mman-common.h.  In order to get the
definitions in such files, arch specific header files must #include the
asm-generic headers.  There are arch specific mmap headers today that do
not include either of the asm-generic headers.  And, they have their own
definitions for MAP_HUGE_SHIFT.  So, it seems we can not use one of the
existing mmap asm-generic header files.  Rather, we would need to create
a new one and have that included by all arch specific files.

However, ALL the MAP_HUGE_* definitions in all the arch specific and
asm-generic header files are the same.  It would be possible to just put
all those MAP_HUGE_* definitions in the primary uapi header file
(include/uapi/linux/mman.h).  If there was ever a need for arch specific
values in the future, we could split them out at that time.

>> Some things to consider.
>>
>> - We should be consistent between mmap and shm.  Also remember, that I
>>   will propose adding the same type of encoding to memfd_create.  So,
>>   three system calls will use the encoding.  They should be consistent.
> 
> agreed
> 
>> - Adding the arch specific definitions seems the 'most correct', as a
>>   user can not use a definition not supported by the arch.  However,
>>   even if an arch supports a huge page size it does not mean that the
>>   running kernel supports that size.  Therefore, the folllowing is in
>>   the man page.
>>   "The  range  of  huge page sizes that are supported by the system
>>    can be discovered by listing  the  subdirectories  in
>>    /sys/kernel/mm/hugepages."
> 
> Doesn't the respective call return -EINVAL on the unsupported hugepage
> size?

Yes, it does.

>> - Another alternative is to make all known huge page sizes available
>>   to all users.  This is 'easier' as the definitions can likely reside
>>   in a common header file.  The user will  need to determine what
>>   huge page sizes are supported by the running kernel as mentioned in
>>   the man page.
> 
> yes I think this makes more sense.

Ok, thanks.

The only remaining question is what kind of common header to use:
1) An asm-generic header file in case there may be arch specific differences
   in the future.
2) Use the primary uapi header file in include/uapi/linux/mman|shm.h.

-- 
Mike Kravetz

[toc] | [prev] | [next] | [standalone]

#1698497

On Thu 27-07-17 14:18:11, Mike Kravetz wrote:
> On 07/27/2017 12:50 AM, Michal Hocko wrote:
> > On Wed 26-07-17 10:39:30, Mike Kravetz wrote:
> >> On 07/26/2017 03:07 AM, Michal Hocko wrote:
> >>> On Wed 26-07-17 11:53:38, Michal Hocko wrote:
> >>>> On Mon 17-07-17 15:28:01, Mike Kravetz wrote:
> >>>>> Use the common definitions from hugetlb_encode.h header file for
> >>>>> encoding hugetlb size definitions in shmget system call flags.  In
> >>>>> addition, move these definitions to the from the internal to user
> >>>>> (uapi) header file.
> >>>>
> >>>> s@to the from@from@
> >>>>
> >>>>>
> >>>>> Suggested-by: Matthew Wilcox <willy@infradead.org>
> >>>>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> >>>>
> >>>> with s@HUGETLB_FLAG_ENCODE__16GB@HUGETLB_FLAG_ENCODE_16GB@
> >>>>
> >>>> Acked-by: Michal Hocko <mhocko@suse.com>
> >>>
> >>> Btw. man page mentions only 2MB and 1GB, we should document others and
> >>> note that each arch might support only subset of them
> >>
> >> Thanks for looking at these Michal.
> >> BTW, those definitions below are wrong.  They should be SHM_HUGE_*. :(
> > 
> > Ups, and I completely missed that.
> > 
> >> In the overview of this RFC, I mentioned still needing to address the
> >> comment from Aneesh about splitting SHM_HUGE_* definitions into arch
> >> specific header files.  This is how it is done for mmap.  If an arch
> >> supports multiple huge page sizes, the 'asm/mman.h' contains definitions
> >> for those sizes.  There will be a bit of churn (such as header file
> >> renaming) to do this for shm as well.  So, I keep going back and forth
> >> asking myself 'is it worth it'?
> > 
> > Why cannot we use a generic header? Btw. I think it would be better for
> > MMAP definitions as well.
> 
> I assume you are asking about a uapi asm-generic header file?  Currently
> mmap has two such files:  mman.h and mman-common.h.  In order to get the
> definitions in such files, arch specific header files must #include the
> asm-generic headers.  There are arch specific mmap headers today that do
> not include either of the asm-generic headers.  And, they have their own
> definitions for MAP_HUGE_SHIFT.  So, it seems we can not use one of the
> existing mmap asm-generic header files.  Rather, we would need to create
> a new one and have that included by all arch specific files.

yes, add a new one like you did in your first patch

> However, ALL the MAP_HUGE_* definitions in all the arch specific and
> asm-generic header files are the same.  It would be possible to just put
> all those MAP_HUGE_* definitions in the primary uapi header file
> (include/uapi/linux/mman.h).  If there was ever a need for arch specific
> values in the future, we could split them out at that time.

agreed

[...]

> >> - Another alternative is to make all known huge page sizes available
> >>   to all users.  This is 'easier' as the definitions can likely reside
> >>   in a common header file.  The user will  need to determine what
> >>   huge page sizes are supported by the running kernel as mentioned in
> >>   the man page.
> > 
> > yes I think this makes more sense.
> 
> Ok, thanks.
> 
> The only remaining question is what kind of common header to use:
> 1) An asm-generic header file in case there may be arch specific differences
>    in the future.
> 2) Use the primary uapi header file in include/uapi/linux/mman|shm.h.

I would use the primary one and only got the arch specific if we ever
need to do arch specific thing.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web