From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753197AbdBJMGi (ORCPT ); Fri, 10 Feb 2017 07:06:38 -0500 Received: from mx2.suse.de ([195.135.220.15]:51559 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751827AbdBJMGh (ORCPT ); Fri, 10 Feb 2017 07:06:37 -0500 Date: Fri, 10 Feb 2017 13:05:24 +0100 From: Michal Hocko To: Hoeun Ryu Cc: Andrew Morton , Ingo Molnar , Andy Lutomirski , Kees Cook , "Eric W. Biederman" , Oleg Nesterov , linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: Re: [PATCH v3] fork: free vmapped stacks in cache when cpus are offline Message-ID: <20170210120524.GI10893@dhcp22.suse.cz> References: <1486715554-12772-1-git-send-email-hoeun.ryu@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1486715554-12772-1-git-send-email-hoeun.ryu@gmail.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 10-02-17 17:32:07, Hoeun Ryu wrote: [...] > +static int free_vm_stack_cache(unsigned int cpu) > +{ > + struct vm_struct **cached_vm_stacks = per_cpu_ptr(cached_stacks, cpu); > + int i; > + > + for (i = 0; i < NR_CACHED_STACKS; i++) { > + struct vm_struct **vm_stack = &cached_vm_stacks[i]; > + > + if (*vm_stack == NULL) > + continue; > + > + vfree((*vm_stack)->addr); > + *vm_stack = NULL; this seems more obscure than necessary. Probably a matter of taste but I would find the following easier to read struct vm_struct *vm_stack = cached_vm_stacks[i]; if (!vm_stack) continue; vfree(vm_stack); cached_vm_stacks[i] = NULL; > + } > + > + return 0; > +} > #endif > > static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node) > @@ -456,6 +474,11 @@ void __init fork_init(void) > for (i = 0; i < UCOUNT_COUNTS; i++) { > init_user_ns.ucount_max[i] = max_threads/2; > } > + > +#ifdef CONFIG_VMAP_STACK > + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "vm_stack_cache", > + NULL, free_vm_stack_cache); > +#endif I am not familiar the new hotplug infrastructure so I might be missing something. CPUHP_AP_ONLINE_DYN will allocate a state which is has only 30 slots available. The name also suggests this will be called on an online event. Why doesn't this have its own state like other users. The name should also reflect offline event CPUHP_STACK_CACHE_DEAD or something like that. -- Michal Hocko SUSE Labs