Re: [PATCH] cgroup: Fixes the un-paired cgroup lock problem

From: Bill Davidsen
Date: Fri Nov 06 2009 - 10:18:20 EST

Li Zefan wrote:
Bill Davidsen wrote:
Li Zefan wrote:
Liu Aleaxander wrote:
From: Liu Aleaxander <Aleaxander@xxxxxxxxx>
Date: Wed, 4 Nov 2009 09:27:06 +0800
Subject: [PATCH] Fixes the un-paired cgroup lock problem

In cgroup_lock_live_group, it locks the cgroup by mutex_lock, while
in the
cgroup_tasks_write, it unlock it by cgroup_unlock. Even though they are
equal, but I do think we should make it pair.

BTW, should we replace others with cgroup_lock and cgroup_unlock?
Since we already have a wrapper one and it's meaningful.

Before I read the email body, I thought there is a bug where
there is a lock without unlock or vise versa.

I agree the case here can be called "unpaired", but I'm not
convinced this patch is needed. The code is not buggy or
confusing. So the patch neither fixes a bug nor make the code
more readable.

I would say it fixes a bug, the one that would be introduced when the
two methods are no longer compatible and essentially two names for the
same thing. And while you may know the code so well that you knew
without looking that this was (currently) okay, there will be lots of
eyes on this code over the years, I think most people would find use of
cgroup_lock to lock the cgroup a LOT more readable.

While you can't go back in time to murder your grandfather, it creates
no paradox to fix a bug before someone writes it.

cgroup_lock() is not necessarily more readable than mutex_lock(&cgroup_mutex),
at least the former doesn't tell you the lock is a spin_lock or a mutex.

That's the point, cgroup_lock() is an abstraction, you want to lock the cgroup, you call the macro, the macro handles the details, and if thinking (or the most common cache configurations) change, the code still works.

In fact, Ingo showed his distaste towards cgroup_lock():

And I won't worry about the issue you mentioned above. If It does
happen, the one, who makes the 2 mehtods no long compatible, will
definitely find out all the places where cgroup_mutex is used and
make proper change.

Bill Davidsen <davidsen@xxxxxxx>
Unintended results are the well-earned reward for incompetence.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at