Pavel Emelyanov wrote:
> [EMAIL PROTECTED] wrote:
>
>>@@ -122,14 +122,26 @@ static void free_pidmap(struct upid *upi
>> atomic_inc(&map->nr_free);
>> }
>>
>>-static int alloc_pidmap(struct pid_namespace *pid_ns)
>>+static int alloc_pidmap(struct pid_namespace *pid_ns, struct pid_list *pid_l,
>>+ int level)
>> {
>> int i, offset, max_scan, pid, last = pid_ns->last_pid;
>> struct pidmap *map;
>>
>>- pid = last + 1;
>>- if (pid >= pid_max)
>>- pid = RESERVED_PIDS;
>>+ if (!pid_l) {
>>+ pid = last + 1;
>>+ if (pid >= pid_max)
>>+ pid = RESERVED_PIDS;
>>+ } else {
>>+ /*
>>+ * There's a target pid, so use it instead
>>+ */
>>+ BUG_ON(level < 0);
>>+ pid = PID_AT(pid_l, level);
>>+ if (pid >= pid_max)
>>+ return -EINVAL;
>>+ }
>>+
>> 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;
>>@@ -153,9 +165,16 @@ static int alloc_pidmap(struct pid_names
>> do {
>> if (!test_and_set_bit(offset, map->page)) {
>> atomic_dec(&map->nr_free);
>>- pid_ns->last_pid = pid;
>>+ if (!pid_l)
>>+ pid_ns->last_pid = pid;
>>+ else
>>+ pid_ns->last_pid = max(last,
>>+ pid);
>> return pid;
>> }
>>+ if (pid_l)
>>+ /* Target pid is already in use */
>>+ return -EBUSY;
>> offset = find_next_offset(map, offset);
>> pid = mk_pid(pid_ns, map, offset);
>> /*
>>@@ -179,7 +198,7 @@ static int alloc_pidmap(struct pid_names
>> }
>> pid = mk_pid(pid_ns, map, offset);
>> }
>>- return -1;
>>+ return -ENOMEM;
>> }
>>
>> int next_pidmap(struct pid_namespace *pid_ns, int last)
>
>
> As fas as this particular piece of code is concerned this all can
> be shrunk down to
>
> static int set_vpidmap(struct pid_namespace *ns, int pid)
> {
> int offset;
> pidmap_t *map;
>
> offset = pid & BITS_PER_PAGE_MASK;
> map = ns->pidmap + vpid / BITS_PER_PAGE;
>
> if (unlikely(alloc_pidmap_page(map)))
> return -ENOMEM;
>
> if (test_and_set_bit(offset, map->page))
> return -EEXIST;
>
> atomic_dec(&map->nr_free);
> return pid;
> }
>
> where the alloc_pidmap_page is a consolidated part of code from alloc_pidmap.
>
> And I'm scared of what the alloc_pid is going to become.
>
>
It's true that I made alloc_pid() become uggly, but this patchset was
more intended to continue a discussion.
What we could do is the following (not compiled, not tested...):
struct pid *alloc_pid(struct pid_namespace *ns)
{
struct pid *pid;
enum pid_type type;
int i, nr;
struct pid_namespace *tmp;
struct upid *upid;
pid = kmem_cache_alloc(ns->pid_cachep, GFP_KERNEL);
if (!pid) {
pid = ERR_PTR(-ENOMEM);
goto out;
}
tmp = ns;
i = ns->level;
if (current->next_id && (current->next_id->flag & SYS_ID_PID)) {
tmp = set_predefined_pids(ns,
current->next_id->pid_ids);
if (IS_ERR(tmp)) {
nr = PTR_ERR(tmp);
goto out_free;
}
}
/*
* Let the lower levels upid nrs be automatically allocated
*/
for ( ; i >= 0; i--) {
nr = alloc_pidmap(tmp, NULL, -1);
if (nr < 0)
goto out_free;
....
which would only add a test and a function call to alloc_pid() ==> more
readable.
with set_predefined_pids defined as follows (still not compiled, not
tested, ...):
struct pid_namespace *set_predefined_pids(struct pid_namespace *ns,
struct pid_list *pid_l)
{
int rel_level;
BUG_ON(!pid_l);
rel_level = pid_l->npids - 1;
if (rel_level > ns->level)
return ERR_PTR(-EINVAL);
/*
* Use the predefined upid nrs for levels ns->level down to
* ns->level - rel_level
*/
for ( ; rel_level >= 0; i--, rel_level--) {
nr = alloc_pidmap(tmp, pid_l, rel_level);
if (nr < 0)
return ERR_PTR(nr);
pid->numbers[i].nr = nr;
pid->numbers[i].ns = tmp;
tmp = tmp->parent;
}
current->next_id->flag &= ~SYS_ID_PID;
pids_free(pid_l);
current->next_id->pid_ids = NULL;
return tmp;
}
Don't you think that mixing this with your 1st proposal (the
set_vpidmap() one), would make things look better?
Regards,
Nadia
_______________________________________________
Containers mailing list
[EMAIL PROTECTED]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
