linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [patch] ACPI: reduce code size, clean up, fix validator message
@ 2006-06-22  8:00 Brown, Len
  2006-06-25 19:54 ` Ingo Molnar
  2006-06-26 16:35 ` Pavel Machek
  0 siblings, 2 replies; 9+ messages in thread
From: Brown, Len @ 2006-06-22  8:00 UTC (permalink / raw)
  To: Ingo Molnar, Andrew Morton
  Cc: michal.k.k.piotrowski, arjan, linux-kernel, linux-acpi, Moore,
	Robert, Arjan van de Ven

Ingo,
Thanks for the quick reply.

An Andrew's advice a while back, Bob already got rid
of the allocate part -- it just isn't upstream yet.

Re: changing ACPICA code (sub-directories of drivers/acpi/) like this:

>-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
>+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);

I can't do that without either
1. diverging between Linux and ACPICA
or
2. getting a license back from you to Intel such that Intel can
   re-distrubute such a change under the Intel license on the file
   and
   inventing spin_lock_irqsave() on about 9 other operating systems.

#1 is all pain and no gain, unless the 244 net fewer bytes counts as
gain.
#2 wouldn't make sense either.

If this code were performance or size critical, I would still delete
acpi_os_acquire_lock from osl.c, but would inline it in aclinux.h.

thanks,
-Len

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch] ACPI: reduce code size, clean up, fix validator message
  2006-06-22  8:00 [patch] ACPI: reduce code size, clean up, fix validator message Brown, Len
@ 2006-06-25 19:54 ` Ingo Molnar
  2006-06-26 16:35 ` Pavel Machek
  1 sibling, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2006-06-25 19:54 UTC (permalink / raw)
  To: Brown, Len
  Cc: Andrew Morton, michal.k.k.piotrowski, arjan, linux-kernel,
	linux-acpi, Moore, Robert, Arjan van de Ven


* Brown, Len <len.brown@intel.com> wrote:

> Ingo,
> Thanks for the quick reply.
> 
> An Andrew's advice a while back, Bob already got rid
> of the allocate part -- it just isn't upstream yet.
> 
> Re: changing ACPICA code (sub-directories of drivers/acpi/) like this:
> 
> >-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
> >+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
> 
> I can't do that without either
> 1. diverging between Linux and ACPICA
> or
> 2. getting a license back from you to Intel such that Intel can
>    re-distrubute such a change under the Intel license on the file and 
>    inventing spin_lock_irqsave() on about 9 other operating systems.

btw., regarding #2 i hereby put my patch (which i wrote in my free time) 
into the public domain - feel free to reuse it in any way, shape or 
form, under any license. (but it's trivial enough so i guess the only 
copyrightable element is my changelog entry anyway ;)

> If this code were performance or size critical, I would still delete 
> acpi_os_acquire_lock from osl.c, but would inline it in aclinux.h.

well its in the kernel so it's size critical by definition. But it's 
certainly not a highprio thing.

	Ingo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch] ACPI: reduce code size, clean up, fix validator message
  2006-06-22  8:00 [patch] ACPI: reduce code size, clean up, fix validator message Brown, Len
  2006-06-25 19:54 ` Ingo Molnar
@ 2006-06-26 16:35 ` Pavel Machek
  1 sibling, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2006-06-26 16:35 UTC (permalink / raw)
  To: Brown, Len
  Cc: Ingo Molnar, Andrew Morton, michal.k.k.piotrowski, arjan,
	linux-kernel, linux-acpi, Moore, Robert, Arjan van de Ven

Hi!

> Thanks for the quick reply.
> 
> An Andrew's advice a while back, Bob already got rid
> of the allocate part -- it just isn't upstream yet.
> 
> Re: changing ACPICA code (sub-directories of drivers/acpi/) like this:
> 
> >-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
> >+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
> 
> I can't do that without either
> 1. diverging between Linux and ACPICA
> or
> 2. getting a license back from you to Intel such that Intel can
>    re-distrubute such a change under the Intel license on the file
>    and
>    inventing spin_lock_irqsave() on about 9 other operating systems.
> 
> #1 is all pain and no gain, unless the 244 net fewer bytes counts as
> gain.
> #2 wouldn't make sense either.

Well, gain here is that code actually becomes readable/linux
like/something.

Feel free to put GPL/BSD license in ACPICA code, saying that by
default contributed code is under both licenses.... or something, but
having linux-like code under drivers/acpi would be great.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [patch] ACPI: reduce code size, clean up, fix validator message
@ 2006-06-26 21:58 Brown, Len
  0 siblings, 0 replies; 9+ messages in thread
From: Brown, Len @ 2006-06-26 21:58 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Pavel Machek, Andrew Morton, michal.k.k.piotrowski, arjan,
	linux-kernel, linux-acpi, Moore, Robert, Arjan van de Ven

 
>( for example the ACPI practice of allocating opaque 'handler' 
> pointers that carry no type at [they are void *] is playing with fire.
It in 
> essence disables the remaining little bit of type-safety 
> that C has. )

I agree 100%.

-Len

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [patch] ACPI: reduce code size, clean up, fix validator message
@ 2006-06-26 20:42 Moore, Robert
  0 siblings, 0 replies; 9+ messages in thread
From: Moore, Robert @ 2006-06-26 20:42 UTC (permalink / raw)
  To: Brown, Len, Pavel Machek
  Cc: Ingo Molnar, Andrew Morton, michal.k.k.piotrowski, arjan,
	linux-kernel, linux-acpi, Arjan van de Ven

Everyone should keep in mind that eventually, the ACPICA code can be
fully integrated into each host operating system and then maintained by
the individual OS projects.

During the time that Intel is actively developing and supporting the
ACPICA code, we need the OS-independent interfaces to provide
portability across the dozen or so host operating systems that are
currently supported.

Yes, of course there is some inefficiency in not using the native OS
interfaces. However, this is really the only sane way to support so many
different hosts, and all OS projects get the benefit of debugging help
and feedback from the many different operating systems.

Bob


> -----Original Message-----
> From: Brown, Len
> Sent: Monday, June 26, 2006 10:40 AM
> To: Pavel Machek
> Cc: Ingo Molnar; Andrew Morton; michal.k.k.piotrowski@gmail.com;
> arjan@linux.intel.com; linux-kernel@vger.kernel.org; linux-
> acpi@vger.kernel.org; Moore, Robert; Arjan van de Ven
> Subject: RE: [patch] ACPI: reduce code size, clean up, fix validator
> message
> 
> 
> >Well, gain here is that code actually becomes readable/linux
> >like/something.
> >
> >Feel free to put GPL/BSD license in ACPICA code, saying that by
> >default contributed code is under both licenses.... or something, but
> >having linux-like code under drivers/acpi would be great.
> 
> There is drivers/acpi/*.c
> This is pure GPL and can be as "Linux like" as any purist wants it to
be.
> Indeed, we have several patches in the queue to do just that in
2.6.18.
> 
> and there is drivers/acpi/*/*.c, which is from ACPICA.
> Linux, along with a bunch of other OS's, is downstream.
> 
> The license on ACPICA is not the issue.
> The issue is when we make a syntax change to ACPICA in Linux,
> then it adds to (my) workload to keep Linux up to date with the
upstream
> ACPICA.
> 
> (note that the previous Linux/ACPI maintainer dealt with this issue
>  by simply over-writing the ACPICA files in Linux upon every update.
>  I allow divergence, but I have to track it, it causes merge
conflicts,
>  and Bob and I actively work to change ACPICA upstream to minimize
it.)
> 
> If you have specific feedback on what can be improved,
> I'm certainly willing to listen.  As you may be aware,
> I translate every ACPICA change into Linux format, and
> it is possible that this process can be enhanced.
> 
> Keep in perspective, however, that we have over 200
> functional issues unresolved in bugzilla.kernel.org,
> and spending time on syntax changes is generally
> a lower priority.
> 
> -Len

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch] ACPI: reduce code size, clean up, fix validator message
  2006-06-26 17:39 Brown, Len
@ 2006-06-26 19:59 ` Ingo Molnar
  0 siblings, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2006-06-26 19:59 UTC (permalink / raw)
  To: Brown, Len
  Cc: Pavel Machek, Andrew Morton, michal.k.k.piotrowski, arjan,
	linux-kernel, linux-acpi, Moore, Robert, Arjan van de Ven


* Brown, Len <len.brown@intel.com> wrote:

> Keep in perspective, however, that we have over 200 functional issues 
> unresolved in bugzilla.kernel.org, and spending time on syntax changes 
> is generally a lower priority.

well, it's your baby and they are your priorities (and i'm really not 
trying to interfere), but still - my personal experience is that syntax 
and functional correctness are strongly connected. I dont claim that 
this particular issue of lock initialization and abstraction is a big 
deal in itself, but cruft does add up over time and becomes a real 
obstacle. I usually spend alot of quality time cleaning up my own code, 
because i know that it directly results in a better ability to improve, 
extend or debug the code in the future. [ Then again, i dont write code 
for 9 platforms :-) ]

( for example the ACPI practice of allocating opaque 'handler' pointers 
  that carry no type at [they are void *] is playing with fire. It in 
  essence disables the remaining little bit of type-safety that C has. )

	Ingo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [patch] ACPI: reduce code size, clean up, fix validator message
@ 2006-06-26 17:39 Brown, Len
  2006-06-26 19:59 ` Ingo Molnar
  0 siblings, 1 reply; 9+ messages in thread
From: Brown, Len @ 2006-06-26 17:39 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Ingo Molnar, Andrew Morton, michal.k.k.piotrowski, arjan,
	linux-kernel, linux-acpi, Moore, Robert, Arjan van de Ven

 
>Well, gain here is that code actually becomes readable/linux
>like/something.
>
>Feel free to put GPL/BSD license in ACPICA code, saying that by
>default contributed code is under both licenses.... or something, but
>having linux-like code under drivers/acpi would be great.

There is drivers/acpi/*.c
This is pure GPL and can be as "Linux like" as any purist wants it to
be.
Indeed, we have several patches in the queue to do just that in 2.6.18.

and there is drivers/acpi/*/*.c, which is from ACPICA.
Linux, along with a bunch of other OS's, is downstream.

The license on ACPICA is not the issue.   
The issue is when we make a syntax change to ACPICA in Linux,
then it adds to (my) workload to keep Linux up to date with the upstream
ACPICA.

(note that the previous Linux/ACPI maintainer dealt with this issue
 by simply over-writing the ACPICA files in Linux upon every update.
 I allow divergence, but I have to track it, it causes merge conflicts,
 and Bob and I actively work to change ACPICA upstream to minimize it.)

If you have specific feedback on what can be improved,
I'm certainly willing to listen.  As you may be aware,
I translate every ACPICA change into Linux format, and
it is possible that this process can be enhanced.

Keep in perspective, however, that we have over 200
functional issues unresolved in bugzilla.kernel.org,
and spending time on syntax changes is generally
a lower priority.

-Len

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch] ACPI: reduce code size, clean up, fix validator message
  2006-06-22  7:20   ` [patch] ACPI: reduce code size, clean up, fix validator message Ingo Molnar
@ 2006-06-22 14:31     ` Michal Piotrowski
  0 siblings, 0 replies; 9+ messages in thread
From: Michal Piotrowski @ 2006-06-22 14:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andrew Morton, Brown, Len, arjan, linux-kernel, linux-acpi,
	robert.moore, Arjan van de Ven

Hi Ingo,

On 22/06/06, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Andrew Morton <akpm@osdl.org> wrote:
>
> > > It complains about this only the 1st time, even though
> > > this same code sequence runs for every (subsequent) ACPI interrupt.
>
> that is because the lock validator turns itself off after the first
> complaint.
>
> > Yes, lockdep uses the callsite of spin_lock_init() to detect the
> > "type" of a lock.
> >
> > But the ACPI obfuscation layers use the same spin_lock_init() site to
> > initialise two not-the-same locks, so lockdep decides those two locks
> > are of the same "type" and gets confused.
> >
> > We had earlier decided to remove that ACPI code which kmallocs a
> > single spinlock.  When that's done, lockdep will become unconfused.
> >
> > AFACIT it's all used for just two statically allocated locks anwyay.
>
> Ok, great! Find below the (tested) cleanup that also fixes the validator
> problem.

Problem fixed, thanks.

>
> (if ACPI wants to turn this into platform-independent code it should be
> a build-time and type-correct translation layer that understands things
> like DEFINE_SPINLOCK as well.)
>
>         Ingo
>

Regards,
Michal

-- 
Michal K. K. Piotrowski
LTG - Linux Testers Group
(http://www.stardust.webpages.pl/ltg/wiki/)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [patch] ACPI: reduce code size, clean up, fix validator message
  2006-06-22  4:59 ` Andrew Morton
@ 2006-06-22  7:20   ` Ingo Molnar
  2006-06-22 14:31     ` Michal Piotrowski
  0 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2006-06-22  7:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Brown, Len, michal.k.k.piotrowski, arjan, linux-kernel,
	linux-acpi, robert.moore, Arjan van de Ven


* Andrew Morton <akpm@osdl.org> wrote:

> > It complains about this only the 1st time, even though
> > this same code sequence runs for every (subsequent) ACPI interrupt.

that is because the lock validator turns itself off after the first 
complaint.

> Yes, lockdep uses the callsite of spin_lock_init() to detect the 
> "type" of a lock.
> 
> But the ACPI obfuscation layers use the same spin_lock_init() site to 
> initialise two not-the-same locks, so lockdep decides those two locks 
> are of the same "type" and gets confused.
> 
> We had earlier decided to remove that ACPI code which kmallocs a 
> single spinlock.  When that's done, lockdep will become unconfused.
> 
> AFACIT it's all used for just two statically allocated locks anwyay.

Ok, great! Find below the (tested) cleanup that also fixes the validator 
problem.

(if ACPI wants to turn this into platform-independent code it should be 
a build-time and type-correct translation layer that understands things 
like DEFINE_SPINLOCK as well.)

	Ingo

----------------------------
Subject: ACPI: reduce code size, clean up, fix validator message
From: Ingo Molnar <mingo@elte.hu>

this patch:

- reduces ACPI code size by 336 bytes:

   text            data    bss     dec           hex      filename
   21848901        6941178 4515464 33305543      1fc33c7  vmlinux-before
   21848565        6941270 4515464 33305299      1fc32d3  vmlinux-after

- cleans the code up by going from the opaque (void *) acpi_handle
  type to an explicit type-checked spinlock_t and by removing 70
  lines of code of unnecessary layering.

- fixes lock validator message by initializing the two static
  locks build-time instead of runtime.

- speeds up the code a bit by reducing extra runtime layering and 
  improving cache footprint.

build and boot tested.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/acpi/events/evgpe.c       |   11 ++++---
 drivers/acpi/events/evgpeblk.c    |   20 ++++++-------
 drivers/acpi/events/evxface.c     |    8 ++---
 drivers/acpi/hardware/hwregs.c    |   19 ++++++------
 drivers/acpi/osl.c                |   56 --------------------------------------
 drivers/acpi/utilities/utglobal.c |    3 ++
 drivers/acpi/utilities/utmutex.c  |   13 --------
 include/acpi/acglobal.h           |    4 +-
 include/acpi/acpiosxf.h           |    8 -----
 9 files changed, 35 insertions(+), 107 deletions(-)

Index: linux/drivers/acpi/events/evgpe.c
===================================================================
--- linux.orig/drivers/acpi/events/evgpe.c
+++ linux/drivers/acpi/events/evgpe.c
@@ -396,7 +396,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_x
 
 	/* We need to hold the GPE lock now, hardware lock in the loop */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Examine all GPE blocks attached to this interrupt level */
 
@@ -413,7 +413,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_x
 
 			gpe_register_info = &gpe_block->register_info[i];
 
-			hw_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
+			spin_lock_irqsave(&acpi_gbl_hardware_lock, hw_flags);
 
 			/* Read the Status Register */
 
@@ -423,7 +423,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_x
 						   &gpe_register_info->
 						   status_address);
 			if (ACPI_FAILURE(status)) {
-				acpi_os_release_lock(acpi_gbl_hardware_lock,
+				spin_unlock_irqrestore(&acpi_gbl_hardware_lock,
 						     hw_flags);
 				goto unlock_and_exit;
 			}
@@ -435,7 +435,8 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_x
 						   &enable_reg,
 						   &gpe_register_info->
 						   enable_address);
-			acpi_os_release_lock(acpi_gbl_hardware_lock, hw_flags);
+			spin_unlock_irqrestore(&acpi_gbl_hardware_lock,
+						hw_flags);
 
 			if (ACPI_FAILURE(status)) {
 				goto unlock_and_exit;
@@ -486,7 +487,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_x
 
       unlock_and_exit:
 
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return (int_status);
 }
 
Index: linux/drivers/acpi/events/evgpeblk.c
===================================================================
--- linux.orig/drivers/acpi/events/evgpeblk.c
+++ linux/drivers/acpi/events/evgpeblk.c
@@ -140,7 +140,7 @@ acpi_status acpi_ev_walk_gpe_list(acpi_g
 
 	ACPI_FUNCTION_TRACE(ev_walk_gpe_list);
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Walk the interrupt level descriptor list */
 
@@ -166,7 +166,7 @@ acpi_status acpi_ev_walk_gpe_list(acpi_g
 	}
 
       unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 
@@ -513,7 +513,7 @@ static struct acpi_gpe_xrupt_info *acpi_
 
 	/* Install new interrupt descriptor with spin lock */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	if (acpi_gbl_gpe_xrupt_list_head) {
 		next_gpe_xrupt = acpi_gbl_gpe_xrupt_list_head;
 		while (next_gpe_xrupt->next) {
@@ -525,7 +525,7 @@ static struct acpi_gpe_xrupt_info *acpi_
 	} else {
 		acpi_gbl_gpe_xrupt_list_head = gpe_xrupt;
 	}
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
 	/* Install new interrupt handler if not SCI_INT */
 
@@ -583,7 +583,7 @@ acpi_ev_delete_gpe_xrupt(struct acpi_gpe
 
 	/* Unlink the interrupt block with lock */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	if (gpe_xrupt->previous) {
 		gpe_xrupt->previous->next = gpe_xrupt->next;
 	}
@@ -591,7 +591,7 @@ acpi_ev_delete_gpe_xrupt(struct acpi_gpe
 	if (gpe_xrupt->next) {
 		gpe_xrupt->next->previous = gpe_xrupt->previous;
 	}
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
 	/* Free the block */
 
@@ -636,7 +636,7 @@ acpi_ev_install_gpe_block(struct acpi_gp
 
 	/* Install the new block at the end of the list with lock */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	if (gpe_xrupt_block->gpe_block_list_head) {
 		next_gpe_block = gpe_xrupt_block->gpe_block_list_head;
 		while (next_gpe_block->next) {
@@ -650,7 +650,7 @@ acpi_ev_install_gpe_block(struct acpi_gp
 	}
 
 	gpe_block->xrupt_block = gpe_xrupt_block;
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
       unlock_and_exit:
 	status = acpi_ut_release_mutex(ACPI_MTX_EVENTS);
@@ -696,7 +696,7 @@ acpi_status acpi_ev_delete_gpe_block(str
 	} else {
 		/* Remove the block on this interrupt with lock */
 
-		flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+		spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 		if (gpe_block->previous) {
 			gpe_block->previous->next = gpe_block->next;
 		} else {
@@ -707,7 +707,7 @@ acpi_status acpi_ev_delete_gpe_block(str
 		if (gpe_block->next) {
 			gpe_block->next->previous = gpe_block->previous;
 		}
-		acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+		spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	}
 
 	/* Free the gpe_block */
Index: linux/drivers/acpi/events/evxface.c
===================================================================
--- linux.orig/drivers/acpi/events/evxface.c
+++ linux/drivers/acpi/events/evxface.c
@@ -613,7 +613,7 @@ acpi_install_gpe_handler(acpi_handle gpe
 
 	/* Install the handler */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	gpe_event_info->dispatch.handler = handler;
 
 	/* Setup up dispatch flags to indicate handler (vs. method) */
@@ -621,7 +621,7 @@ acpi_install_gpe_handler(acpi_handle gpe
 	gpe_event_info->flags &= ~(ACPI_GPE_XRUPT_TYPE_MASK | ACPI_GPE_DISPATCH_MASK);	/* Clear bits */
 	gpe_event_info->flags |= (u8) (type | ACPI_GPE_DISPATCH_HANDLER);
 
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
       unlock_and_exit:
 	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
@@ -707,7 +707,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_
 
 	/* Remove the handler */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	handler = gpe_event_info->dispatch.handler;
 
 	/* Restore Method node (if any), set dispatch flags */
@@ -717,7 +717,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_
 	if (handler->method_node) {
 		gpe_event_info->flags |= ACPI_GPE_DISPATCH_METHOD;
 	}
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
 	/* Now we can free the handler object */
 
Index: linux/drivers/acpi/hardware/hwregs.c
===================================================================
--- linux.orig/drivers/acpi/hardware/hwregs.c
+++ linux/drivers/acpi/hardware/hwregs.c
@@ -67,7 +67,7 @@ ACPI_MODULE_NAME("hwregs")
 acpi_status acpi_hw_clear_acpi_status(u32 flags)
 {
 	acpi_status status;
-	acpi_cpu_flags lock_flags = 0;
+	acpi_cpu_flags lock_flags;
 
 	ACPI_FUNCTION_TRACE(hw_clear_acpi_status);
 
@@ -75,7 +75,7 @@ acpi_status acpi_hw_clear_acpi_status(u3
 			  ACPI_BITMASK_ALL_FIXED_STATUS,
 			  (u16) acpi_gbl_FADT->xpm1a_evt_blk.address));
 
-	lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
+	spin_lock_irqsave(&acpi_gbl_hardware_lock, lock_flags);
 
 	status = acpi_hw_register_write(ACPI_MTX_DO_NOT_LOCK,
 					ACPI_REGISTER_PM1_STATUS,
@@ -100,7 +100,8 @@ acpi_status acpi_hw_clear_acpi_status(u3
 	status = acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block);
 
       unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
+	spin_unlock_irqrestore(&acpi_gbl_hardware_lock, lock_flags);
+
 	return_ACPI_STATUS(status);
 }
 
@@ -339,7 +340,7 @@ acpi_status acpi_set_register(u32 regist
 		return_ACPI_STATUS(AE_BAD_PARAMETER);
 	}
 
-	lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
+	spin_lock_irqsave(&acpi_gbl_hardware_lock, lock_flags);
 
 	/* Always do a register read first so we can insert the new bits  */
 
@@ -447,7 +448,7 @@ acpi_status acpi_set_register(u32 regist
 
       unlock_and_exit:
 
-	acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
+	spin_unlock_irqrestore(&acpi_gbl_hardware_lock, lock_flags);
 
 	/* Normalize the value that was read */
 
@@ -488,7 +489,7 @@ acpi_hw_register_read(u8 use_lock, u32 r
 	ACPI_FUNCTION_TRACE(hw_register_read);
 
 	if (ACPI_MTX_LOCK == use_lock) {
-		lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
+		spin_lock_irqsave(&acpi_gbl_hardware_lock, lock_flags);
 	}
 
 	switch (register_id) {
@@ -566,7 +567,7 @@ acpi_hw_register_read(u8 use_lock, u32 r
 
       unlock_and_exit:
 	if (ACPI_MTX_LOCK == use_lock) {
-		acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
+		spin_unlock_irqrestore(&acpi_gbl_hardware_lock, lock_flags);
 	}
 
 	if (ACPI_SUCCESS(status)) {
@@ -599,7 +600,7 @@ acpi_status acpi_hw_register_write(u8 us
 	ACPI_FUNCTION_TRACE(hw_register_write);
 
 	if (ACPI_MTX_LOCK == use_lock) {
-		lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
+		spin_lock_irqsave(&acpi_gbl_hardware_lock, lock_flags);
 	}
 
 	switch (register_id) {
@@ -689,7 +690,7 @@ acpi_status acpi_hw_register_write(u8 us
 
       unlock_and_exit:
 	if (ACPI_MTX_LOCK == use_lock) {
-		acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
+		spin_unlock_irqrestore(&acpi_gbl_hardware_lock, lock_flags);
 	}
 
 	return_ACPI_STATUS(status);
Index: linux/drivers/acpi/osl.c
===================================================================
--- linux.orig/drivers/acpi/osl.c
+++ linux/drivers/acpi/osl.c
@@ -685,40 +685,6 @@ void acpi_os_wait_events_complete(void *
 
 EXPORT_SYMBOL(acpi_os_wait_events_complete);
 
-/*
- * Allocate the memory for a spinlock and initialize it.
- */
-acpi_status acpi_os_create_lock(acpi_handle * out_handle)
-{
-	spinlock_t *lock_ptr;
-
-	ACPI_FUNCTION_TRACE("os_create_lock");
-
-	lock_ptr = acpi_os_allocate(sizeof(spinlock_t));
-
-	spin_lock_init(lock_ptr);
-
-	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Creating spinlock[%p].\n", lock_ptr));
-
-	*out_handle = lock_ptr;
-
-	return_ACPI_STATUS(AE_OK);
-}
-
-/*
- * Deallocate the memory for a spinlock.
- */
-void acpi_os_delete_lock(acpi_handle handle)
-{
-	ACPI_FUNCTION_TRACE("os_create_lock");
-
-	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Deleting spinlock[%p].\n", handle));
-
-	acpi_os_free(handle);
-
-	return_VOID;
-}
-
 acpi_status
 acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle)
 {
@@ -1037,28 +1003,6 @@ unsigned int max_cstate = ACPI_PROCESSOR
 
 EXPORT_SYMBOL(max_cstate);
 
-/*
- * Acquire a spinlock.
- *
- * handle is a pointer to the spinlock_t.
- */
-
-acpi_cpu_flags acpi_os_acquire_lock(acpi_handle handle)
-{
-	acpi_cpu_flags flags;
-	spin_lock_irqsave((spinlock_t *) handle, flags);
-	return flags;
-}
-
-/*
- * Release a spinlock. See above.
- */
-
-void acpi_os_release_lock(acpi_handle handle, acpi_cpu_flags flags)
-{
-	spin_unlock_irqrestore((spinlock_t *) handle, flags);
-}
-
 #ifndef ACPI_USE_LOCAL_CACHE
 
 /*******************************************************************************
Index: linux/drivers/acpi/utilities/utglobal.c
===================================================================
--- linux.orig/drivers/acpi/utilities/utglobal.c
+++ linux/drivers/acpi/utilities/utglobal.c
@@ -46,6 +46,9 @@
 #include <acpi/acpi.h>
 #include <acpi/acnamesp.h>
 
+DEFINE_SPINLOCK(acpi_gbl_gpe_lock);
+DEFINE_SPINLOCK(acpi_gbl_hardware_lock);
+
 #define _COMPONENT          ACPI_UTILITIES
 ACPI_MODULE_NAME("utglobal")
 
Index: linux/drivers/acpi/utilities/utmutex.c
===================================================================
--- linux.orig/drivers/acpi/utilities/utmutex.c
+++ linux/drivers/acpi/utilities/utmutex.c
@@ -79,15 +79,6 @@ acpi_status acpi_ut_mutex_initialize(voi
 			return_ACPI_STATUS(status);
 		}
 	}
-
-	/* Create the spinlocks for use at interrupt level */
-
-	status = acpi_os_create_lock(&acpi_gbl_gpe_lock);
-	if (ACPI_FAILURE(status)) {
-		return_ACPI_STATUS(status);
-	}
-
-	status = acpi_os_create_lock(&acpi_gbl_hardware_lock);
 	return_ACPI_STATUS(status);
 }
 
@@ -116,10 +107,6 @@ void acpi_ut_mutex_terminate(void)
 		(void)acpi_ut_delete_mutex(i);
 	}
 
-	/* Delete the spinlocks */
-
-	acpi_os_delete_lock(acpi_gbl_gpe_lock);
-	acpi_os_delete_lock(acpi_gbl_hardware_lock);
 	return_VOID;
 }
 
Index: linux/include/acpi/acglobal.h
===================================================================
--- linux.orig/include/acpi/acglobal.h
+++ linux/include/acpi/acglobal.h
@@ -317,8 +317,8 @@ ACPI_EXTERN struct acpi_gpe_block_info
 
 /* Spinlocks */
 
-ACPI_EXTERN acpi_handle acpi_gbl_gpe_lock;
-ACPI_EXTERN acpi_handle acpi_gbl_hardware_lock;
+extern spinlock_t acpi_gbl_gpe_lock;
+extern spinlock_t acpi_gbl_hardware_lock;
 
 /*****************************************************************************
  *
Index: linux/include/acpi/acpiosxf.h
===================================================================
--- linux.orig/include/acpi/acpiosxf.h
+++ linux/include/acpi/acpiosxf.h
@@ -108,14 +108,6 @@ acpi_status acpi_os_wait_semaphore(acpi_
 
 acpi_status acpi_os_signal_semaphore(acpi_handle handle, u32 units);
 
-acpi_status acpi_os_create_lock(acpi_handle * out_handle);
-
-void acpi_os_delete_lock(acpi_handle handle);
-
-acpi_cpu_flags acpi_os_acquire_lock(acpi_handle handle);
-
-void acpi_os_release_lock(acpi_handle handle, acpi_cpu_flags flags);
-
 /*
  * Memory allocation and mapping
  */

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2006-06-26 22:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-06-22  8:00 [patch] ACPI: reduce code size, clean up, fix validator message Brown, Len
2006-06-25 19:54 ` Ingo Molnar
2006-06-26 16:35 ` Pavel Machek
  -- strict thread matches above, loose matches on Subject: below --
2006-06-26 21:58 Brown, Len
2006-06-26 20:42 Moore, Robert
2006-06-26 17:39 Brown, Len
2006-06-26 19:59 ` Ingo Molnar
2006-06-22  4:28 2.6.17-mm1 - possible recursive locking detected Brown, Len
2006-06-22  4:59 ` Andrew Morton
2006-06-22  7:20   ` [patch] ACPI: reduce code size, clean up, fix validator message Ingo Molnar
2006-06-22 14:31     ` Michal Piotrowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).