| Message ID | 20240827-patches-below_hint_mmap-v1-7-46ff2eb9022d@rivosinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | mm: Introduce MAP_BELOW_HINT | expand |
Hi Charlie, Le 28/08/2024 à 07:49, Charlie Jenkins a écrit : > Add support for MAP_BELOW_HINT to arch_get_mmap_base() and > arch_get_mmap_end(). > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > --- > arch/powerpc/include/asm/task_size_64.h | 36 +++++++++++++++++++++++++++------ > 1 file changed, 30 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/include/asm/task_size_64.h b/arch/powerpc/include/asm/task_size_64.h > index 239b363841aa..a37a5a81365d 100644 > --- a/arch/powerpc/include/asm/task_size_64.h > +++ b/arch/powerpc/include/asm/task_size_64.h > @@ -72,12 +72,36 @@ > #define STACK_TOP_MAX TASK_SIZE_USER64 > #define STACK_TOP (is_32bit_task() ? STACK_TOP_USER32 : STACK_TOP_USER64) > > -#define arch_get_mmap_base(addr, len, base, flags) \ > - (((addr) > DEFAULT_MAP_WINDOW) ? (base) + TASK_SIZE - DEFAULT_MAP_WINDOW : (base)) > +#define arch_get_mmap_base(addr, len, base, flags) \ This macro looks quite big for a macro, can it be a static inline function instead ? Same for the other macro below. > +({ \ > + unsigned long mmap_base; \ > + typeof(flags) _flags = (flags); \ > + typeof(addr) _addr = (addr); \ > + typeof(base) _base = (base); \ > + typeof(len) _len = (len); \ > + unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \ > + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1)))\ > + mmap_base = (_addr + _len) - rnd_gap; \ > + else \ > + mmap_end = ((_addr > DEFAULT_MAP_WINDOW) ? \ > + _base + TASK_SIZE - DEFAULT_MAP_WINDOW : \ > + _base); \ > + mmap_end; \ mmap_end doesn't exist, did you mean mmap_base ? > +}) > > -#define arch_get_mmap_end(addr, len, flags) \ > - (((addr) > DEFAULT_MAP_WINDOW) || \ > - (((flags) & MAP_FIXED) && ((addr) + (len) > DEFAULT_MAP_WINDOW)) ? TASK_SIZE : \ > - DEFAULT_MAP_WINDOW) > +#define arch_get_mmap_end(addr, len, flags) \ > +({ \ > + unsigned long mmap_end; \ > + typeof(flags) _flags = (flags); \ > + typeof(addr) _addr = (addr); \ > + typeof(len) _len = (len); \ > + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1))) \ > + mmap_end = (_addr + _len); \ > + else \ > + mmap_end = (((_addr) > DEFAULT_MAP_WINDOW) || \ > + (((_flags) & MAP_FIXED) && ((_addr) + (_len) > DEFAULT_MAP_WINDOW))\ > + ? TASK_SIZE : DEFAULT_MAP_WINDOW) \ > + mmap_end; \ > +}) > > #endif /* _ASM_POWERPC_TASK_SIZE_64_H */ >
On Wed, Aug 28, 2024 at 08:34:49AM +0200, Christophe Leroy wrote: > Hi Charlie, > > Le 28/08/2024 à 07:49, Charlie Jenkins a écrit : > > Add support for MAP_BELOW_HINT to arch_get_mmap_base() and > > arch_get_mmap_end(). > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > arch/powerpc/include/asm/task_size_64.h | 36 +++++++++++++++++++++++++++------ > > 1 file changed, 30 insertions(+), 6 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/task_size_64.h b/arch/powerpc/include/asm/task_size_64.h > > index 239b363841aa..a37a5a81365d 100644 > > --- a/arch/powerpc/include/asm/task_size_64.h > > +++ b/arch/powerpc/include/asm/task_size_64.h > > @@ -72,12 +72,36 @@ > > #define STACK_TOP_MAX TASK_SIZE_USER64 > > #define STACK_TOP (is_32bit_task() ? STACK_TOP_USER32 : STACK_TOP_USER64) > > -#define arch_get_mmap_base(addr, len, base, flags) \ > > - (((addr) > DEFAULT_MAP_WINDOW) ? (base) + TASK_SIZE - DEFAULT_MAP_WINDOW : (base)) > > +#define arch_get_mmap_base(addr, len, base, flags) \ > > This macro looks quite big for a macro, can it be a static inline function > instead ? Same for the other macro below. > I had overlooked that possibility, I think that's a great solution, I will change that. > > +({ \ > > + unsigned long mmap_base; \ > > + typeof(flags) _flags = (flags); \ > > + typeof(addr) _addr = (addr); \ > > + typeof(base) _base = (base); \ > > + typeof(len) _len = (len); \ > > + unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \ > > + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1)))\ > > + mmap_base = (_addr + _len) - rnd_gap; \ > > + else \ > > + mmap_end = ((_addr > DEFAULT_MAP_WINDOW) ? \ > > + _base + TASK_SIZE - DEFAULT_MAP_WINDOW : \ > > + _base); \ > > + mmap_end; \ > > mmap_end doesn't exist, did you mean mmap_base ? Oh whoops, thank you! - Charlie > > > +}) > > -#define arch_get_mmap_end(addr, len, flags) \ > > - (((addr) > DEFAULT_MAP_WINDOW) || \ > > - (((flags) & MAP_FIXED) && ((addr) + (len) > DEFAULT_MAP_WINDOW)) ? TASK_SIZE : \ > > - DEFAULT_MAP_WINDOW) > > +#define arch_get_mmap_end(addr, len, flags) \ > > +({ \ > > + unsigned long mmap_end; \ > > + typeof(flags) _flags = (flags); \ > > + typeof(addr) _addr = (addr); \ > > + typeof(len) _len = (len); \ > > + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1))) \ > > + mmap_end = (_addr + _len); \ > > + else \ > > + mmap_end = (((_addr) > DEFAULT_MAP_WINDOW) || \ > > + (((_flags) & MAP_FIXED) && ((_addr) + (_len) > DEFAULT_MAP_WINDOW))\ > > + ? TASK_SIZE : DEFAULT_MAP_WINDOW) \ > > + mmap_end; \ > > +}) > > #endif /* _ASM_POWERPC_TASK_SIZE_64_H */ > >
From: Christophe Leroy > Sent: 28 August 2024 07:35 > Hi Charlie, > > Le 28/08/2024 à 07:49, Charlie Jenkins a écrit : > > Add support for MAP_BELOW_HINT to arch_get_mmap_base() and > > arch_get_mmap_end(). > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > arch/powerpc/include/asm/task_size_64.h | 36 +++++++++++++++++++++++++++------ > > 1 file changed, 30 insertions(+), 6 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/task_size_64.h b/arch/powerpc/include/asm/task_size_64.h > > index 239b363841aa..a37a5a81365d 100644 > > --- a/arch/powerpc/include/asm/task_size_64.h > > +++ b/arch/powerpc/include/asm/task_size_64.h > > @@ -72,12 +72,36 @@ > > #define STACK_TOP_MAX TASK_SIZE_USER64 > > #define STACK_TOP (is_32bit_task() ? STACK_TOP_USER32 : STACK_TOP_USER64) > > > > -#define arch_get_mmap_base(addr, len, base, flags) \ > > - (((addr) > DEFAULT_MAP_WINDOW) ? (base) + TASK_SIZE - DEFAULT_MAP_WINDOW : (base)) > > +#define arch_get_mmap_base(addr, len, base, flags) \ > > This macro looks quite big for a macro, can it be a static inline > function instead ? Same for the other macro below. Or even a real function? Given the actual cost of mapping memory an extra function call isn't going to be masurable. David > > > +({ \ > > + unsigned long mmap_base; \ > > + typeof(flags) _flags = (flags); \ > > + typeof(addr) _addr = (addr); \ > > + typeof(base) _base = (base); \ > > + typeof(len) _len = (len); \ > > + unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \ > > + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1)))\ > > + mmap_base = (_addr + _len) - rnd_gap; \ > > + else \ > > + mmap_end = ((_addr > DEFAULT_MAP_WINDOW) ? \ > > + _base + TASK_SIZE - DEFAULT_MAP_WINDOW : \ > > + _base); \ > > + mmap_end; \ > > mmap_end doesn't exist, did you mean mmap_base ? > > > +}) > > > > -#define arch_get_mmap_end(addr, len, flags) \ > > - (((addr) > DEFAULT_MAP_WINDOW) || \ > > - (((flags) & MAP_FIXED) && ((addr) + (len) > DEFAULT_MAP_WINDOW)) ? TASK_SIZE : \ > > - DEFAULT_MAP_WINDOW) > > +#define arch_get_mmap_end(addr, len, flags) \ > > +({ \ > > + unsigned long mmap_end; \ > > + typeof(flags) _flags = (flags); \ > > + typeof(addr) _addr = (addr); \ > > + typeof(len) _len = (len); \ > > + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1))) \ > > + mmap_end = (_addr + _len); \ > > + else \ > > + mmap_end = (((_addr) > DEFAULT_MAP_WINDOW) || \ > > + (((_flags) & MAP_FIXED) && ((_addr) + (_len) > DEFAULT_MAP_WINDOW))\ > > + ? TASK_SIZE : DEFAULT_MAP_WINDOW) \ > > + mmap_end; \ > > +}) > > > > #endif /* _ASM_POWERPC_TASK_SIZE_64_H */ > > - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/arch/powerpc/include/asm/task_size_64.h b/arch/powerpc/include/asm/task_size_64.h index 239b363841aa..a37a5a81365d 100644 --- a/arch/powerpc/include/asm/task_size_64.h +++ b/arch/powerpc/include/asm/task_size_64.h @@ -72,12 +72,36 @@ #define STACK_TOP_MAX TASK_SIZE_USER64 #define STACK_TOP (is_32bit_task() ? STACK_TOP_USER32 : STACK_TOP_USER64) -#define arch_get_mmap_base(addr, len, base, flags) \ - (((addr) > DEFAULT_MAP_WINDOW) ? (base) + TASK_SIZE - DEFAULT_MAP_WINDOW : (base)) +#define arch_get_mmap_base(addr, len, base, flags) \ +({ \ + unsigned long mmap_base; \ + typeof(flags) _flags = (flags); \ + typeof(addr) _addr = (addr); \ + typeof(base) _base = (base); \ + typeof(len) _len = (len); \ + unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \ + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1)))\ + mmap_base = (_addr + _len) - rnd_gap; \ + else \ + mmap_end = ((_addr > DEFAULT_MAP_WINDOW) ? \ + _base + TASK_SIZE - DEFAULT_MAP_WINDOW : \ + _base); \ + mmap_end; \ +}) -#define arch_get_mmap_end(addr, len, flags) \ - (((addr) > DEFAULT_MAP_WINDOW) || \ - (((flags) & MAP_FIXED) && ((addr) + (len) > DEFAULT_MAP_WINDOW)) ? TASK_SIZE : \ - DEFAULT_MAP_WINDOW) +#define arch_get_mmap_end(addr, len, flags) \ +({ \ + unsigned long mmap_end; \ + typeof(flags) _flags = (flags); \ + typeof(addr) _addr = (addr); \ + typeof(len) _len = (len); \ + if (_flags & MAP_BELOW_HINT && _addr != 0 && ((_addr + _len) > BIT(VA_BITS - 1))) \ + mmap_end = (_addr + _len); \ + else \ + mmap_end = (((_addr) > DEFAULT_MAP_WINDOW) || \ + (((_flags) & MAP_FIXED) && ((_addr) + (_len) > DEFAULT_MAP_WINDOW))\ + ? TASK_SIZE : DEFAULT_MAP_WINDOW) \ + mmap_end; \ +}) #endif /* _ASM_POWERPC_TASK_SIZE_64_H */
Add support for MAP_BELOW_HINT to arch_get_mmap_base() and arch_get_mmap_end(). Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- arch/powerpc/include/asm/task_size_64.h | 36 +++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-)