From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751908AbdJCQPO (ORCPT ); Tue, 3 Oct 2017 12:15:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32924 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751528AbdJCQPN (ORCPT ); Tue, 3 Oct 2017 12:15:13 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 477A183F40 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=oleg@redhat.com Date: Tue, 3 Oct 2017 18:15:10 +0200 From: Oleg Nesterov To: akpm@linux-foundation.org Cc: adobriyan@gmail.com, ebiederm@xmission.com, keescook@chromium.org, mm-commits@vger.kernel.org, linux-kernel@vger.kernel.org, Gargi Sharma , Rik van Riel Subject: Re: + pid-delete-struct-pidmap-nr_free.patch added to -mm tree Message-ID: <20171003161510.GB9929@redhat.com> References: <59d2c81b.sN3iiDcgxwFHtFRg%akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <59d2c81b.sN3iiDcgxwFHtFRg%akpm@linux-foundation.org> User-Agent: Mutt/1.5.24 (2015-08-30) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 03 Oct 2017 16:15:13 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/02, Andrew Morton wrote: > > From: Alexey Dobriyan > Subject: pid: delete struct pidmap::nr_free > > There is a check in pid allocation code to skip a full page: > > if (likely(atomic_read(&map->nr_free))) { > ... > > In practice it doesn't do anything. To skip a pidmap page one has to have > 32K consecutive pids allocated at the same time which doesn't happen. > > Currently the price is _every_ fork/exit on every system being slower than > necessary. Agreed, I too never understood how can this counter help. Add Gargi and Rik, the next version of "Replace PID bitmap allocation with IDR API" can conflict with this and the previous pid-delete-reserved_pids.patch. Oleg. > > Link: http://lkml.kernel.org/r/20170909203649.GB4791@avx2 > Signed-off-by: Alexey Dobriyan > Cc: "Eric W. Biederman" > Cc: Kees Cook > Cc: Oleg Nesterov > Signed-off-by: Andrew Morton > --- > > include/linux/pid_namespace.h | 1 - > kernel/pid.c | 28 ++++++++++------------------ > kernel/pid_namespace.c | 6 ------ > 3 files changed, 10 insertions(+), 25 deletions(-) > > diff -puN include/linux/pid_namespace.h~pid-delete-struct-pidmap-nr_free include/linux/pid_namespace.h > --- a/include/linux/pid_namespace.h~pid-delete-struct-pidmap-nr_free > +++ a/include/linux/pid_namespace.h > @@ -11,7 +11,6 @@ > #include > > struct pidmap { > - atomic_t nr_free; > void *page; > }; > > diff -puN kernel/pid.c~pid-delete-struct-pidmap-nr_free kernel/pid.c > --- a/kernel/pid.c~pid-delete-struct-pidmap-nr_free > +++ a/kernel/pid.c > @@ -68,9 +68,6 @@ static inline int mk_pid(struct pid_name > */ > struct pid_namespace init_pid_ns = { > .kref = KREF_INIT(2), > - .pidmap = { > - [ 0 ... PIDMAP_ENTRIES-1] = { ATOMIC_INIT(BITS_PER_PAGE), NULL } > - }, > .last_pid = 0, > .nr_hashed = PIDNS_HASH_ADDING, > .level = 0, > @@ -106,7 +103,6 @@ static void free_pidmap(struct upid *upi > int offset = nr & BITS_PER_PAGE_MASK; > > clear_bit(offset, map->page); > - atomic_inc(&map->nr_free); > } > > /* > @@ -181,20 +177,17 @@ static int alloc_pidmap(struct pid_names > if (unlikely(!map->page)) > return -ENOMEM; > } > - if (likely(atomic_read(&map->nr_free))) { > - for ( ; ; ) { > - if (!test_and_set_bit(offset, map->page)) { > - atomic_dec(&map->nr_free); > - set_last_pid(pid_ns, last, pid); > - return pid; > - } > - offset = find_next_offset(map, offset); > - if (offset >= BITS_PER_PAGE) > - break; > - pid = mk_pid(pid_ns, map, offset); > - if (pid >= pid_max) > - break; > + for (;;) { > + if (!test_and_set_bit(offset, map->page)) { > + set_last_pid(pid_ns, last, pid); > + return pid; > } > + offset = find_next_offset(map, offset); > + if (offset >= BITS_PER_PAGE) > + break; > + pid = mk_pid(pid_ns, map, offset); > + if (pid >= pid_max) > + break; > } > if (map < &pid_ns->pidmap[(pid_max-1)/BITS_PER_PAGE]) { > ++map; > @@ -591,7 +584,6 @@ void __init pidmap_init(void) > init_pid_ns.pidmap[0].page = kzalloc(PAGE_SIZE, GFP_KERNEL); > /* Reserve PID 0. We never call free_pidmap(0) */ > set_bit(0, init_pid_ns.pidmap[0].page); > - atomic_dec(&init_pid_ns.pidmap[0].nr_free); > > init_pid_ns.pid_cachep = KMEM_CACHE(pid, > SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT); > diff -puN kernel/pid_namespace.c~pid-delete-struct-pidmap-nr_free kernel/pid_namespace.c > --- a/kernel/pid_namespace.c~pid-delete-struct-pidmap-nr_free > +++ a/kernel/pid_namespace.c > @@ -98,7 +98,6 @@ static struct pid_namespace *create_pid_ > struct pid_namespace *ns; > unsigned int level = parent_pid_ns->level + 1; > struct ucounts *ucounts; > - int i; > int err; > > err = -EINVAL; > @@ -139,11 +138,6 @@ static struct pid_namespace *create_pid_ > INIT_WORK(&ns->proc_work, proc_cleanup_work); > > set_bit(0, ns->pidmap[0].page); > - atomic_set(&ns->pidmap[0].nr_free, BITS_PER_PAGE - 1); > - > - for (i = 1; i < PIDMAP_ENTRIES; i++) > - atomic_set(&ns->pidmap[i].nr_free, BITS_PER_PAGE); > - > return ns; > > out_free_map: > _ > > Patches currently in -mm which might be from adobriyan@gmail.com are > > proc-uninline-name_to_int.patch > proc-use-do-while-in-name_to_int.patch > seq_file-delete-small-value-optimization.patch > pid-delete-reserved_pids.patch > pid-delete-struct-pidmap-nr_free.patch >