From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932302AbdJWPbl (ORCPT ); Mon, 23 Oct 2017 11:31:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46012 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932102AbdJWPbk (ORCPT ); Mon, 23 Oct 2017 11:31:40 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DAB26369C9 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=oleg@redhat.com Date: Mon, 23 Oct 2017 17:31:36 +0200 From: Oleg Nesterov To: Gargi Sharma Cc: Andrei Vagin , linux-kernel@vger.kernel.org, Rik van Riel , Julia Lawall , Andrew Morton , mingo@kernel.org, pasha.tatashin@oracle.com, ktkhai@virtuozzo.com, "Eric W. Biederman" , Christoph Hellwig , lkp@intel.com, tony.luck@intel.com Subject: Re: [v6,1/2] pid: Replace pid bitmap implementation with IDR API Message-ID: <20171023153136.GA27553@redhat.com> References: <1507760379-21662-2-git-send-email-gs051095@gmail.com> <20171019073050.GC29091@outlook.office365.com> <20171019161800.GA8886@redhat.com> <20171020172100.GA29093@outlook.office365.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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.30]); Mon, 23 Oct 2017 15:31:40 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/22, Gargi Sharma wrote: > > On Fri, Oct 20, 2017 at 6:21 PM, Andrei Vagin wrote: > > On Fri, Oct 20, 2017 at 05:06:47PM +0100, Gargi Sharma wrote: > >> On Thu, Oct 19, 2017 at 5:18 PM, Oleg Nesterov wrote: > >> > > >> > unsigned int last; > >> > int err; > >> > > >> > tmp.data = &last; > >> > err = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos); > >> > if (!err) > >> > idr_set_cursor(&pid_ns->idr, last + 1); > >> > return err; > >> I'm not sure entirely understand how this takes care of rolling over of PIDs? > >> Can we ignore that? If yes, won't the tests for CRIU still break? > > > > Gargi, I don't understand what you mean. Could you elaborate? Do you > > mean a case when idr_next is bigger than pid_max? I think this logic > > remains the same what we had before switching to idr. > > When the PIDs are allocated, if the allocation exceeds pid_max wraps > around and starts allocating PIDs starting from pid_min. You misunderstood the problem introduced by your patch... We do not care about pid_max overlap. The problem is that criu writes to /proc/sys/kernel/ns_last_pid to control the pid number allocated by the next fork(). So if you do "echo 100 > /proc/sys/kernel/ns_last_pid" the next fork() should create the process with tid=101 (of course, if 101 is free). After your patch the new task will have tid=100. See? > > CRIU tests works with a following patch. It is slightly modified version > > of Oleg's patch. Andrei, could you write the changelog and send the fix to akpm? Feel free to add my ack or sob. Oleg. > > diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c > > index fea2c24..1c791b3 100644 > > --- a/kernel/pid_namespace.c > > +++ b/kernel/pid_namespace.c > > @@ -287,6 +287,7 @@ static int pid_ns_ctl_handler(struct ctl_table > > *table, int write, > > { > > struct pid_namespace *pid_ns = task_active_pid_ns(current); > > struct ctl_table tmp = *table; > > + int ret; > > > > if (write && !ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN)) > > return -EPERM; > > @@ -298,7 +299,12 @@ static int pid_ns_ctl_handler(struct ctl_table > > *table, int write, > > */ > > > > tmp.data = &pid_ns->idr.idr_next; > > - return proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos); > > + ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos); > > + if (ret < 0) > > + return ret; > > + > > + idr_set_cursor(&pid_ns->idr, pid_ns->idr.idr_next + 1); > > + return 0; > > } > > > > extern int pid_max; > > > > > >> > >> Thanks, > >> Gargi > >> > > >> > Oleg. > >> >