From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
To: frankeh@watson.ibm.com
Cc: "Rajan Ravindran" <rajancr@us.ibm.com>,
linux-kernel@vger.kernel.org, lse-tech@lists.sourceforge.net
Subject: Re: [PATCH] get_pid() performance fix
Date: Sat, 16 Mar 2002 00:16:48 +0900 [thread overview]
Message-ID: <87663xlv33.fsf@devron.myhome.or.jp> (raw)
In-Reply-To: <OF810580E6.8672B341-ON85256B73.005AF9B8@pok.ibm.com> <20020314231733.638C03FE06@smtp.linux.ibm.com>
In-Reply-To: <20020314231733.638C03FE06@smtp.linux.ibm.com>
Whoops! I'm sorry. previous email was the middle of writing.
Hubertus Franke <frankeh@watson.ibm.com> writes:
> + if (i == PID_MAP_SIZE) {
> + if (again) {
> + /* we didn't find any pid , sweep and try again */
> + again = 0;
> + memset(pid_map, 0, PID_MAP_SIZE * sizeof(unsigned long));
> + last_pid = RESERVED_PIDS;
> + goto repeat;
> + }
> + next_safe = RESERVED_PIDS;
> + return 0;
Probably, the bug is here.
the following is test case: ./getpid1 -r300 -c3
case 3:
populate_all(0, 1);
pid = get_pid(0);
printf("new pid: %d\n", pid);
tsk = find_task_by_pid(400);
del_task(tsk);
pid = get_pid(0);
printf("new pid: %d\n", pid);
break;
result,
new pid: 0
new pid: 1
> + }
> +
> + fpos = ffz(mask);
> + i &= (PID_MAX-1);
> + last_pid = (i << SHIFT_PER_LONG) + fpos;
> +
> + /* find next save pid */
> + mask &= ~((1 << fpos) - 1);
> +
> + while ((mask == 0) && (++i < PID_MAP_SIZE))
> + mask = pid_map[i];
> +
> + if (i==PID_MAP_SIZE)
> + next_safe = PID_MAX;
> + else
> + next_safe = (i << SHIFT_PER_LONG) + ffs(mask) - 1;
> + return last_pid;
> +}
> +
> static int get_pid(unsigned long flags)
> {
> - static int next_safe = PID_MAX;
> struct task_struct *p;
> - int pid;
> + int pid,beginpid;
>
> if (flags & CLONE_PID)
> return current->pid;
>
> spin_lock(&lastpid_lock);
> + beginpid = last_pid;
> if((++last_pid) & 0xffff8000) {
> - last_pid = 300; /* Skip daemons etc. */
> + last_pid = RESERVED_PIDS; /* Skip daemons etc. */
> goto inside;
> }
> if(last_pid >= next_safe) {
> inside:
> next_safe = PID_MAX;
> read_lock(&tasklist_lock);
> + if (nr_threads > GETPID_THRESHOLD) {
> + last_pid = get_pid_by_map(last_pid);
> + } else {
> repeat:
> for_each_task(p) {
> if(p->pid == last_pid ||
> @@ -151,9 +228,11 @@
> p->session == last_pid) {
> if(++last_pid >= next_safe) {
> if(last_pid & 0xffff8000)
> - last_pid = 300;
> + last_pid = RESERVED_PIDS;
> next_safe = PID_MAX;
> }
> + if(unlikely(last_pid == beginpid))
> + goto nomorepids;
> goto repeat;
> }
> if(p->pid > last_pid && next_safe > p->pid)
> @@ -162,6 +241,9 @@
> next_safe = p->pgrp;
> if(p->session > last_pid && next_safe > p->session)
> next_safe = p->session;
> + if(p->tgid > last_pid && next_safe > p->tgid)
> + next_safe = p->tgid;
> + }
> }
> read_unlock(&tasklist_lock);
> }
> @@ -169,6 +251,10 @@
> spin_unlock(&lastpid_lock);
>
> return pid;
> +nomorepids:
Probably, the following line are required, here.
+ next_safe = RESERVED_PIDS; /* or 0 */
> + read_unlock(&tasklist_lock);
> + spin_unlock(&lastpid_lock);
> + return 0;
> }
Basically nice, I think.
BTW, How about using the __set_bit(), find_next_zero_bit(), and
find_next_bit() in get_pid_by_map().
Thanks for nice work.
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
next prev parent reply other threads:[~2002-03-15 15:17 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2002-03-05 16:43 Fwd: [Lse-tech] get_pid() performance fix Rajan Ravindran
2002-03-05 17:57 ` OGAWA Hirofumi
2002-03-05 19:53 ` Hubertus Franke
2002-03-05 20:10 ` OGAWA Hirofumi
2002-03-05 21:59 ` Hubertus Franke
2002-03-05 22:48 ` OGAWA Hirofumi
2002-03-05 23:40 ` Hubertus Franke
2002-03-14 23:18 ` [PATCH] " Hubertus Franke
2002-03-15 14:57 ` OGAWA Hirofumi
2002-03-15 15:16 ` OGAWA Hirofumi [this message]
2002-03-15 18:37 ` Hubertus Franke
2002-03-16 5:12 ` OGAWA Hirofumi
2002-03-18 21:44 ` Hubertus Franke
2002-03-22 22:14 ` Hubertus Franke
2002-03-22 22:28 ` Davide Libenzi
2002-03-22 22:26 ` Hubertus Franke
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=87663xlv33.fsf@devron.myhome.or.jp \
--to=hirofumi@mail.parknet.co.jp \
--cc=frankeh@watson.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lse-tech@lists.sourceforge.net \
--cc=rajancr@us.ibm.com \
/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;
as well as URLs for NNTP newsgroup(s).