[patch] ACPI: reduce code size, clean up, fix validator message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Andrew Morton <akpm@xxxxxxxx> 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@xxxxxxx>

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@xxxxxxx>
---
 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
  */
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux