On 14/10/15 13:12, Jungseok Lee wrote: > On Oct 14, 2015, at 12:00 AM, Jungseok Lee wrote: >> On Oct 13, 2015, at 8:00 PM, James Morse wrote: >>> On 12/10/15 23:13, Jungseok Lee wrote: >>>> On Oct 13, 2015, at 1:34 AM, James Morse wrote: >>>>> Having two kmem_caches for 16K stacks on a 64K page system may be wasteful >>>>> (especially for systems with few cpus)… >>>> >>>> This would be a single concern. To address this issue, I drop the 'static' >>>> keyword in thread_info_cache. Please refer to the below hunk. >>> >>> Its only a problem on systems with 64K pages, which don't have a multiple >>> of 4 cpus. I suspect if you turn on 64K pages, you have many cores with >>> plenty of memory… >> >> Yes, the problem 'two kmem_caches' comes from only 64K page system. >> >> I don't get the statement 'which don't have a multiple of 4 cpus'. >> Could you point out what I am missing? > > You're talking about sl{a|u}b allocator behavior. If so, I got what you meant.
Yes, With Nx4 cpus, the (currently) 16K irq stacks take up Nx64K - a nice multiple of pages, so no wasted memory. >>> If this has been made a published symbol, it should go in a header file. >> >> Sure. > > I had the wrong impression that there is a room under include/linux/*. Yes, I see there isn't anywhere obvious to put it... > IMO, this is architectural option whether arch relies on thread_info_cache or > not. > In other words, it would be clear to put this extern under > arch/*/include/asm/*. Its up to the arch whether or not to define CONFIG_ARCH_THREAD_INFO_ALLOCATOR. In the case where it hasn't defined it, and THREAD_SIZE >= PAGE_SIZE, your change is exposing thread_info_cache on all architectures, so it ought go in a header file accessible to all architectures. Something like this, (only build-tested!): =========%<========= --- a/include/linux/thread_info.h +++ b/include/linux/thread_info.h @@ -10,6 +10,8 @@ #include <linux/types.h> #include <linux/bug.h> +#include <asm/page.h> + struct timespec; struct compat_timespec; @@ -145,6 +147,12 @@ static inline bool test_and_clear_restore_sigmask(void) #error "no set_restore_sigmask() provided and default one won't work" #endif +#ifndef CONFIG_ARCH_THREAD_INFO_ALLOCATOR +#if THREAD_SIZE >= PAGE_SIZE +extern struct kmem_cache *thread_info_cache; +#endif /* THREAD_SIZE >= PAGE_SIZE */ +#endif /* CONFIG_ARCH_THREAD_INFO_ALLOCATOR */ + #endif /* __KERNEL__ */ #endif /* _LINUX_THREAD_INFO_H */ =========%<========= Quite ugly! My concern is there could be push-back from the maintainer of kernel/fork.c, saying "define CONFIG_ARCH_THREAD_INFO_ALLOCATOR if the generic code isn't what you need", and push-back from the arm64 maintainers about copy-pasting that chunk into arch/arm64.... both of which are fair, hence my initial version created a second kmem_cache. Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/