public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Matt Helsley <matthltc@us.ibm.com>,
	Oren Laadan <orenl@librato.com>,
	Daniel Lezcano <daniel.lezcano@free.fr>,
	randy.dunlap@oracle.com, arnd@arndb.de,
	linux-api@vger.kernel.org,
	Containers <containers@lists.linux-foundation.org>,
	Nathan Lynch <nathanl@austin.ibm.com>,
	linux-kernel@vger.kernel.org, Louis.Rilling@kerlabs.com,
	kosaki.motohiro@jp.fujitsu.com, hpa@zytor.com, mingo@elte.hu,
	torvalds@linux-foundation.org,
	Alexey Dobriyan <adobriyan@gmail.com>,
	roland@redhat.com, Pavel Emelyanov <xemul@openvz.org>
Subject: Re: [RFC][v8][PATCH 0/10] Implement clone3() system call
Date: Wed, 21 Oct 2009 02:16:36 -0700	[thread overview]
Message-ID: <m1eioxrtsb.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <20091021062021.GA2667@us.ibm.com> (Sukadev Bhattiprolu's message of "Tue\, 20 Oct 2009 23\:20\:21 -0700")

Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> writes:

> Eric W. Biederman [ebiederm@xmission.com] wrote:
> | > I will post a version of the patch outside this patchset with min
> | > and max parameters and we can see if it can be optimized/beautified.
> | 
> | Thanks,
> | Eric
>
> Here is the patch.  alloc_pid() calls alloc_pidmap() as follows:
>
> -		nr = alloc_pidmap(tmp);
> +		min = max = 0;
> +		if (target_pids) {
> +			min = target_pids[i];
> +			max = min + 1;
> +		}
> +		nr = alloc_pidmap(tmp, min, max);
>
> It does look like this patch executes a bit more code even in the common
> case but let me know if you think this is better.
> 	
> ---

Not what I was thinking.  The following untested patch is what I was
thinking.  It just exposes last, min, and max to the callers which pass
in different appropriate values.

Eric

diff --git a/kernel/pid.c b/kernel/pid.c
index d3f722d..d9b6f82 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -122,17 +122,17 @@ static void free_pidmap(struct upid *upid)
 	atomic_inc(&map->nr_free);
 }
 
-static int alloc_pidmap(struct pid_namespace *pid_ns)
+static int do_alloc_pidmap(struct pid_namespace *pid_ns, int last, int min, int max)
 {
-	int i, offset, max_scan, pid, last = pid_ns->last_pid;
+	int i, offset, max_scan, pid;
 	struct pidmap *map;
 
 	pid = last + 1;
-	if (pid >= pid_max)
-		pid = RESERVED_PIDS;
+	if (pid >= max)
+		pid = min;
 	offset = pid & BITS_PER_PAGE_MASK;
 	map = &pid_ns->pidmap[pid/BITS_PER_PAGE];
-	max_scan = (pid_max + BITS_PER_PAGE - 1)/BITS_PER_PAGE - !offset;
+	max_scan = (max + BITS_PER_PAGE - 1)/BITS_PER_PAGE - !offset;
 	for (i = 0; i <= max_scan; ++i) {
 		if (unlikely(!map->page)) {
 			void *page = kzalloc(PAGE_SIZE, GFP_KERNEL);
@@ -153,7 +153,6 @@ static int alloc_pidmap(struct pid_namespace *pid_ns)
 			do {
 				if (!test_and_set_bit(offset, map->page)) {
 					atomic_dec(&map->nr_free);
-					pid_ns->last_pid = pid;
 					return pid;
 				}
 				offset = find_next_offset(map, offset);
@@ -164,16 +163,16 @@ static int alloc_pidmap(struct pid_namespace *pid_ns)
 			 * bitmap block and the final block was the same
 			 * as the starting point, pid is before last_pid.
 			 */
-			} while (offset < BITS_PER_PAGE && pid < pid_max &&
+			} while (offset < BITS_PER_PAGE && pid < max &&
 					(i != max_scan || pid < last ||
 					    !((last+1) & BITS_PER_PAGE_MASK)));
 		}
-		if (map < &pid_ns->pidmap[(pid_max-1)/BITS_PER_PAGE]) {
+		if (map < &pid_ns->pidmap[(max-1)/BITS_PER_PAGE]) {
 			++map;
 			offset = 0;
 		} else {
 			map = &pid_ns->pidmap[0];
-			offset = RESERVED_PIDS;
+			offset = min;
 			if (unlikely(last == offset))
 				break;
 		}
@@ -182,6 +181,25 @@ static int alloc_pidmap(struct pid_namespace *pid_ns)
 	return -1;
 }
 
+static int alloc_pidmap(struct pid_namespace *pid_ns)
+{
+	int nr;
+
+	nr = do_alloc_pidmap(pid_ns, pid_ns->last, RESERVED_PIDS, pid_max);
+	if (nr >= 0)
+		pid_ns->last_pid = nr;
+	return nr;
+}
+
+static int set_pidmap(struct pid_namespace *pid_ns, int target)
+{
+	if (target >= pid_max)
+		return -1;
+	if (target < RESERVED_PIDS)
+		return -1;
+	return do_alloc_pidmap(pid_ns, target - 1, target, target + 1);
+}
+
 int next_pidmap(struct pid_namespace *pid_ns, int last)
 {
 	int offset;

  reply	other threads:[~2009-10-21  9:16 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-13  4:49 [RFC][v8][PATCH 0/10] Implement clone3() system call Sukadev Bhattiprolu
2009-10-13  4:49 ` [RFC][v8][PATCH 1/10]: Factor out code to allocate pidmap page Sukadev Bhattiprolu
2009-10-13  4:50 ` [RFC][v8][PATCH 2/10]: Have alloc_pidmap() return actual error code Sukadev Bhattiprolu
2009-10-13  4:50 ` [RFC][v8][PATCH 3/10]: Make pid_max a pid_ns property Sukadev Bhattiprolu
2009-10-13  5:19   ` Alexey Dobriyan
2009-10-13 13:09   ` Pavel Emelyanov
2009-10-13 15:24     ` Serge E. Hallyn
2009-10-13 16:10       ` Pavel Emelyanov
2009-10-13 16:28         ` Serge E. Hallyn
2009-10-13  4:51 ` [RFC][v8][PATCH 4/10]: Add target_pid parameter to alloc_pidmap() Sukadev Bhattiprolu
2009-10-13 11:50   ` Pavel Emelyanov
2009-10-15  0:24     ` Sukadev Bhattiprolu
2009-10-13  4:51 ` [RFC][v8][PATCH 5/10]: Add target_pids parameter to alloc_pid() Sukadev Bhattiprolu
2009-10-13  4:52 ` [RFC][v8][PATCH 6/10]: Add target_pids parameter to copy_process() Sukadev Bhattiprolu
2009-10-13  4:52 ` [RFC][v8][PATCH 7/10]: Check invalid clone flags Sukadev Bhattiprolu
2009-10-13 18:35   ` Oren Laadan
2009-10-13 23:38     ` Sukadev Bhattiprolu
2009-10-13  4:52 ` [RFC][v8][PATCH 8/10]: Define do_fork_with_pids() Sukadev Bhattiprolu
2009-10-13  4:54 ` [RFC][v8][PATCH 9/10]: Define clone3() syscall Sukadev Bhattiprolu
2009-10-13 18:46   ` Oren Laadan
2009-10-16  4:20   ` Sukadev Bhattiprolu
2009-10-16  6:25     ` Michael Kerrisk
2009-10-16 18:06       ` Sukadev Bhattiprolu
2009-10-19 17:44         ` Matt Helsley
2009-10-19 21:31           ` H. Peter Anvin
2009-10-19 23:50             ` Matt Helsley
2009-10-21  4:26               ` Michael Kerrisk
2009-10-21 13:03                 ` H. Peter Anvin
2009-10-21 19:44                   ` Sukadev Bhattiprolu
2009-10-21 22:03                     ` H. Peter Anvin
2009-10-22 10:40                     ` Michael Kerrisk
2009-10-22 18:10                       ` Sukadev Bhattiprolu
2009-10-22 10:26                   ` Michael Kerrisk
2009-10-22 11:38                     ` H. Peter Anvin
2009-10-22 12:14                       ` Michael Kerrisk
2009-10-22 12:19                         ` H. Peter Anvin
2009-10-22 13:57                         ` Matt Helsley
2009-10-13  4:55 ` [RFC][v8][PATCH 10/10]: Document " Sukadev Bhattiprolu
2009-10-14 12:26   ` Arnd Bergmann
2009-10-14 18:39     ` Sukadev Bhattiprolu
2009-10-19 21:36   ` Pavel Machek
2009-10-21  8:37     ` Arnd Bergmann
2009-10-21  9:33       ` Pavel Machek
2009-10-21 13:26         ` Arnd Bergmann
2009-10-21 18:27     ` Sukadev Bhattiprolu
2009-10-13 20:50 ` [RFC][v8][PATCH 0/10] Implement clone3() system call Roland McGrath
2009-10-13 23:27   ` Sukadev Bhattiprolu
2009-10-13 23:53     ` Roland McGrath
2009-10-14  1:13       ` H. Peter Anvin
2009-10-14  4:36         ` Sukadev Bhattiprolu
2009-10-14  4:38           ` H. Peter Anvin
2009-10-14 22:36         ` Sukadev Bhattiprolu
2009-10-14 22:49           ` H. Peter Anvin
2009-10-15  0:17             ` Sukadev Bhattiprolu
2009-10-13 23:49 ` H. Peter Anvin
2009-10-14  1:39   ` Matt Helsley
2009-10-14  2:24     ` H. Peter Anvin
2009-10-14  4:40       ` Sukadev Bhattiprolu
2009-10-14  4:50         ` H. Peter Anvin
2009-10-14 16:07         ` Serge E. Hallyn
2009-10-16 19:22 ` Daniel Lezcano
2009-10-16 19:44   ` Sukadev Bhattiprolu
2009-10-19 20:34     ` Daniel Lezcano
2009-10-19 21:47       ` Oren Laadan
2009-10-20  0:51         ` Matt Helsley
2009-10-20  3:33           ` Eric W. Biederman
2009-10-20  4:03             ` Sukadev Bhattiprolu
2009-10-20 10:46               ` Eric W. Biederman
2009-10-20 14:16                 ` Serge E. Hallyn
2009-10-20 18:33                 ` Sukadev Bhattiprolu
2009-10-20 19:26                   ` Eric W. Biederman
2009-10-20 20:13                     ` Oren Laadan
2009-10-21  6:20                     ` Sukadev Bhattiprolu
2009-10-21  9:16                       ` Eric W. Biederman [this message]
2009-10-21 18:52                         ` Sukadev Bhattiprolu
2009-10-21 21:11                           ` Eric W. Biederman
2009-10-23  0:42                         ` Sukadev Bhattiprolu
2009-10-23  1:03                           ` Eric W. Biederman
2009-10-23  5:30                             ` Sukadev Bhattiprolu
2009-10-23  5:44                               ` Eric W. Biederman
2009-10-23 19:21                                 ` Sukadev Bhattiprolu
2009-10-23 20:48                                   ` Sukadev Bhattiprolu
2009-10-23 23:26                                     ` Eric W. Biederman
2009-10-24  3:38                                       ` Sukadev Bhattiprolu
2009-10-23 19:16                               ` Oren Laadan
2009-10-23 19:34                                 ` Oren Laadan
2009-10-23 23:12                                   ` Eric W. Biederman
2009-10-20 14:09             ` Serge E. Hallyn
2009-10-21 15:53         ` Daniel Lezcano
2009-10-21 18:45           ` Oren Laadan
2009-10-22 11:22             ` Daniel Lezcano
  -- strict thread matches above, loose matches on Subject: below --
2009-10-26  9:38 Albert Cahalan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1eioxrtsb.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=Louis.Rilling@kerlabs.com \
    --cc=adobriyan@gmail.com \
    --cc=arnd@arndb.de \
    --cc=containers@lists.linux-foundation.org \
    --cc=daniel.lezcano@free.fr \
    --cc=hpa@zytor.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthltc@us.ibm.com \
    --cc=mingo@elte.hu \
    --cc=nathanl@austin.ibm.com \
    --cc=orenl@librato.com \
    --cc=randy.dunlap@oracle.com \
    --cc=roland@redhat.com \
    --cc=sukadev@linux.vnet.ibm.com \
    --cc=torvalds@linux-foundation.org \
    --cc=xemul@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox