linux-kernel - Re: [PATCH 1/3] cgroup: convert open-coded mutex_lock(&cgroup_mutex) calls into cgroup

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]

Message-ID: <6599ad830901191728k102f586ar1f252b8604226d95@mail.gmail.com>
Date:	Mon, 19 Jan 2009 17:28:48 -0800
From:	Paul Menage <menage@...gle.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Lai Jiangshan <laijs@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	miaox@...fujitsu.com, maxk@...lcomm.com,
	linux-kernel@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH 1/3] cgroup: convert open-coded mutex_lock(&cgroup_mutex) 
	calls into cgroup_lock() calls

On Sun, Jan 18, 2009 at 5:41 PM, Ingo Molnar <mingo@...e.hu> wrote:
>
> * Paul Menage <menage@...gle.com> wrote:
>
>> On Sun, Jan 18, 2009 at 1:10 AM, Ingo Molnar <mingo@...e.hu> wrote:
>> > this just changes over a clean mutex call to a wrapped lock/unlock
>> > sequence that has higher overhead in the common case.
>> >
>> > We should do the exact opposite, we should change this opaque API:
>> >
>> >  void cgroup_lock(void)
>> >  {
>> >         mutex_lock(&cgroup_mutex);
>> >  }
>> >
>> > To something more explicit (and more maintainable) like:
>>
>> I disagree - cgroup_mutex is a very coarse lock that can be held for
>> pretty long periods of time by the cgroups framework, and should never
>> be part of any fastpath code. So the overhead of a function call should
>> be irrelevant.
>>
>> The change that you're proposing would send the message that
>> cgroup_mutex_lock(&cgroup_mutex) is appropriate to use in a
>> performance-sensitive function, when in fact we want to discourage such
>> code from taking this lock and instead use more appropriately
>> fine-grained locks.
>
> Uhm, how does that 'discourage' its use in fastpath code?

I agree, the existing code doesn't exactly discourage its use in
fastpath code, but we should be doing so. (The recent addition of
hierarchy_mutex is a step in that direction, although it still has
some issues to be cleaned up). But it seems to me that exposing the
lock is an invitation for people to use it even more than they do
currently. There's certainly no performance argument for exposing it,

>
> It just hides the real lock

Yes, I'd rather hide the real lock in this case.

Paul
--
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/