lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 04 Nov 2009 09:26:14 -0500
From:	Bill Davidsen <davidsen@....com>
To:	Li Zefan <lizf@...fujitsu.com>
CC:	Liu Aleaxander <aleaxander@...il.com>,
	Paul Menage <menage@...gle.com>, linux-kernel@...r.kernel.org,
	containers@...ts.osdl.org
Subject: Re: [PATCH] cgroup: Fixes the un-paired cgroup lock problem

Li Zefan wrote:
> Liu Aleaxander wrote:
>> From: Liu Aleaxander <Aleaxander@...il.com>
>> 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.

-- 
Bill Davidsen <davidsen@....com>
   "We have more to fear from the bungling of the incompetent than from
the machinations of the wicked."  - from Slashdot
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ