Re: [PATCH] cpuset semaphore depth check optimize

Roman Zippel
2005-09-13T11:43:16Z

Hi,

On Mon, 12 Sep 2005, Paul Jackson wrote:

> There are two reasons a cpuset is accessed from within the
> allocation code.
> 1) Update the per-task mems_allowed if the current tasks cpuset
> has changed its memory placement due to some other task
> independently modifying that cpuset.
> 2) Walk up the cpuset hierarchy, starting from the tasks
> cpuset, looking for a cpuset that is marked mem_exclusive,
> and using the mems_allowed from that exclusive cpuset.

If I read the source correctly, a cpuset cannot be removed or moved while
it's attached to a task, which makes it a lot simpler.

This means you have to take the second lock when accessing tsk->cpuset
(you can basically replace task_lock()). Any allocator callback can now
use the second lock to scan the cpusets. IOW as soon as count is different
from zero, the cpuset is active and certain members become read-only.
Activation/deactivation is controlled by the second lock.

You can BTW avoid locking in cpuset_exit() completely in the common case:

tsk->cpuset = NULL;
if (atomic_dec_and_test(&cs->count) && notify_on_release(cs)) {
...
}

Here you only need to release the reference, noone else should use that
task anymore. You only have to check in attach_task() that you don't
change a dead task.

There may be a subtle problem with cpuset_fork(), there is a window from
dup_task_struct() until cpuset_fork(), where we have two pointers to a
cpuset but only a single reference. Another process could now change the
cpuset of the parent process to a different cpuset and the child process
may end up with an invalid cpuset and I don't see how this protected. The
only (simple) solution I see is to do this:

lock();
tsk->cpuset = current->cpuset;
atomic_inc(&tsk->cpuset->count);
unlock();

bye, Roman