Messages in this thread | | | Date | Mon, 11 Mar 2013 14:57:34 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 1/1] kernel/pid.c: Improve flow of a loop inside alloc_pidmap. |
| |
On Mon, 11 Mar 2013 18:18:56 -0300 "Raphael S. Carvalho" <raphael.scarv@gmail.com> wrote:
> From: Raphael S.Carvalho <raphael.scarv@gmail.com> > > Notes: find_next_offset searches for an available "cleaned bit" > in the respective pid bitmap (page), so returns the offset if found, > otherwise it returns a value equals to BITS_PER_PAGE. > > For example, suppose find_next_offset didn't find any available > bit, so there's no purpose to call mk_pid (Wasteful Cpu Cycles). > > Therefore, I found it could be better to call mk_pid after > the checking (offset < BITS_PER_PAGE) returned sucessfully! > Another point: If (offset < BITS_PER_PAGE) results in a "failure", > then mk_pid would be called again afterwards. > > ... > > --- a/kernel/pid.c > +++ b/kernel/pid.c > @@ -190,8 +190,8 @@ static int alloc_pidmap(struct pid_namespace *pid_ns) > return pid; > } > offset = find_next_offset(map, offset); > - pid = mk_pid(pid_ns, map, offset); > - } while (offset < BITS_PER_PAGE && pid < pid_max); > + } while (offset < BITS_PER_PAGE && > + (pid = mk_pid(pid_ns, map, offset)) < pid_max); > } > if (map < &pid_ns->pidmap[(pid_max-1)/BITS_PER_PAGE]) { > ++map;
Looks OK. But I think it's simpler and more straightforward to do it this way?
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; }
| |