public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <ak@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Zhao Yakui <yakui.zhao@intel.com>,
	Daniel Walker <dwalker@mvista.com>,
	linux-acpi@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	len.brown@intel.com,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Matthew Wilcox <matthew@wil.cx>
Subject: Re: [PATCH 1/3] acpi: add real mutex function calls
Date: Mon, 21 Jul 2008 22:00:49 +0200	[thread overview]
Message-ID: <4884EAF1.4050901@linux.intel.com> (raw)
In-Reply-To: <1216668835.7257.91.camel@twins>

Peter Zijlstra wrote:
> On Mon, 2008-07-21 at 21:15 +0200, Andi Kleen wrote:
>> Peter Zijlstra wrote:
>>> On Mon, 2008-07-21 at 11:17 +0200, Andi Kleen wrote:
>>>> Peter Zijlstra wrote:
>>>>> On Mon, 2008-07-21 at 09:51 +0800, Zhao Yakui wrote:
>>>>>> On Sat, 2008-07-19 at 11:16 -0700, Daniel Walker wrote:
>>>>>>> Instead of re-using semaphores for the mutex operation, I've
>>>>>>> added usage of the kernel mutex for the os mutex implementation.
>>>>>>>
>>>>>> What is the advantage that the kernel mutex is used for the ACPI mutex
>>>>>> implementation instead of using semaphore?
>>>>>> And it seems that too much ACPICA source code is touched.
>>>>> You get help from lockdep, and also our goal is to fully eradicate
>>>>> semaphore usage.
>>>> Issue is that ACPICA is shared with other OS source code and to replace 
>>>> a major interface like this would mean replacing it for everyone. It 
>>>> might end up with ACPICA just reimplementing a semaphore like wrapper if 
>>>> semaphores really go away, but I don't really see that coming anyways.
>>> Andi, you know better than that.
>> Know better than what?
> 
> Know better than to say we need ugly code in Linux because
> $SOME_OTHER_OS.

What ugly code do you refer to?

Ok ACPICA has its dark corners (I won't deny that), but at least willy's 
new semaphore code cannot really be described as "ugly".

>> My understanding was that there are a few areas
>> in the kernel who really use true semaphore semantics and I don't see it 
>> as particularly useful to force them to use something else that doesn't 
>> fit them as well.  And there are areas like ACPICA where semaphores are 
>> an useful abstraction because of other consideration (in ACPICA's case 
>> due to portability).
> 
> Does ACPICA use counting semaphores? If so, you could have used real
> arguments against his patches, instead of this other-os bull.

It is how ACPICA is maintained. You can yell at that as much as you want
and call it "bull", but that won't change it.

> Also, what is the justification for using counting semaphores? Are we
> counting hardware slots or is it just generic ACPI braindamage?

ACPICA is portable code and that needs a reasonably wide spread locking
abstraction. Happens to be semaphores for various reasons.

Now if mutexes were possible (AFAIK that's not possible in the general
case) then that would work too. But using "completions" as the low
level API doesn't make sense to me because it'll just lead to higher
level locking code being reinvented and also because it's unlikely
anything else has those with these semantics.

I think what would make sense is to identify the locks in ACPICA
which only have mutex semantics and convert them to a new osl mutex 
abstraction. Then at least some of the locks could get lockdep
checking.

On the other hand to be honest I'm not looking forward to handle
the usual 90+% false positives that you get out of lockdep ...

> Clearly this all wasn't extremely clear from the code - otherwise Daniel
> wouldn't even have done these patches.
> 
>> Especially now that semaphores are not duplicated per architecture 
>> anymore so actually keeping them around is not that costly.
> 
> Having them around might give people the idea its a good idea to use
> them. Not having them around is a good way to discourage that.

Code reviewers will take care of that.

-Andi

  parent reply	other threads:[~2008-07-21 20:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-19 18:16 [PATCH 0/3] acpi: acpi: sem out, mutex/completion in Daniel Walker
2008-07-19 18:16 ` [PATCH 1/3] acpi: add real mutex function calls Daniel Walker
2008-07-19 18:16   ` [PATCH 2/3] acpi: semaphore removal Daniel Walker
2008-07-19 18:16     ` [PATCH 3/3] Add lockdep integration for the ACPI mutex usage Daniel Walker
2008-07-20  8:15     ` [PATCH 2/3] acpi: semaphore removal Dave Chinner
2008-07-20 14:49       ` Daniel Walker
2008-07-21  1:51   ` [PATCH 1/3] acpi: add real mutex function calls Zhao Yakui
2008-07-21  9:14     ` Peter Zijlstra
2008-07-21  9:17       ` Andi Kleen
2008-07-21  9:24         ` Peter Zijlstra
2008-07-21 19:15           ` Andi Kleen
2008-07-21 19:33             ` Peter Zijlstra
2008-07-21 19:55               ` Matthew Wilcox
2008-07-21 20:22                 ` Daniel Walker
2008-07-21 20:00               ` Andi Kleen [this message]
2008-07-21 20:38                 ` Daniel Walker
2008-07-21 13:59         ` Daniel Walker
2008-07-21 13:56     ` Daniel Walker
2008-07-21 19:20       ` Andi Kleen
2008-07-21 19:39         ` Daniel Walker
2008-07-23 22:14           ` Moore, Robert
2008-07-24 12:44             ` Daniel Walker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4884EAF1.4050901@linux.intel.com \
    --to=ak@linux.intel.com \
    --cc=dwalker@mvista.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=yakui.zhao@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox