Re: [PATCH] cpuset semaphore depth check optimize

[Roman Zippel <zippel@linux-m68k.org>]

Hi,

On Tue, 13 Sep 2005, Paul Jackson wrote:

> > 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.
> 
> Yes - a cpuset cannot be removed while attached (count > 0).  And there
> is no 'move' that I know of.  A rename(2) system call on a cpuset in
> the cpuset filesystem gets the vfs default -EINVAL response.
> 
> So, yes, if I can pin a cpuset with a per-cpuset spinlock on it, then
> its parent chain (and whatever else I take care to guard with that
> spinlock) is held as well (the cpuset->parent chain is pinned).  I
> guess this some of what you meant by your phrase "makes it a lot
> simpler".

I don't think a per-cpuset spinlock will be necessary (at least 
initially).
The complete active condition is actually (atomic_read(&cs->count) || 
!list_empty(&cs->children)). These means if any child is possibly active 
so is the parent. 
Modifications in the cpuset hierarchy require the cpuset_sem and an 
inactive cpuset, (de)activating a cpuset requires the cpuset_sem and 
(let's call it) cpuset_tasklock.
Callbacks from the allocator now only need cpuset_tasklock to access the 
cpuset via tsk->cpuset and to keep it active and an active cpuset can't be 
removed from the hierarchy.

> You also wrote:
> > 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)) {
> 
> I don't think that works.  And I suspect you are proposing the same bug
> that I had, and fixed with the following patch:

You're right, it should better look like this:

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

This way it only may happen that two notifaction are sent.

bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/