On Wed, 2013-02-13 at 18:21 +0100, Thomas Gleixner wrote:
> On Wed, 13 Feb 2013, John Kacur wrote:
> >
> > Thanks Steven. That looks way better than the previous revert.
>
> I can't tell as I haven't seen the previous revert. And looks good is
> not really a good review criteria.
I agreed, and warned John on IRC about that. This is why the patch had
RFC in the subject.
>
> The patch is converting _all_ the spin_locks in acpi to raw spinlocks,
> which will give you a nice bunch of "BUG: sleeping function called
> from invalid context" splats depending on the ACPI functionality of
> your machine.
Yep, I asked to have this tested on a wide range of boxes with the debug
options enabled. Hoping to see if splats do happen.
>
> The lock which is related to this splat is: acpi_gbl_hardware_lock and
> that's the only lock which can be safely converted to a raw spinlock.
>
> Untested patch below.
I'll try this patch instead, and see what breaks ;-)
Thanks,
-- Steve
>
> Thanks,
>
> tglx
>
>
> Index: linux-stable/drivers/acpi/acpica/acglobal.h
> ===================================================================
> --- linux-stable.orig/drivers/acpi/acpica/acglobal.h
> +++ linux-stable/drivers/acpi/acpica/acglobal.h
> @@ -251,7 +251,7 @@ ACPI_EXTERN u8 acpi_gbl_global_lock_pend
> * interrupt level
> */
> ACPI_EXTERN acpi_spinlock acpi_gbl_gpe_lock; /* For GPE data structs and
> registers */
> -ACPI_EXTERN acpi_spinlock acpi_gbl_hardware_lock; /* For ACPI H/W except
> GPE registers */
> +ACPI_EXTERN acpi_raw_spinlock acpi_gbl_hardware_lock; /* For ACPI H/W
> except GPE registers */
>
>
> /*****************************************************************************
> *
> Index: linux-stable/drivers/acpi/acpica/hwregs.c
> ===================================================================
> --- linux-stable.orig/drivers/acpi/acpica/hwregs.c
> +++ linux-stable/drivers/acpi/acpica/hwregs.c
> @@ -271,14 +271,14 @@ acpi_status acpi_hw_clear_acpi_status(vo
> ACPI_BITMASK_ALL_FIXED_STATUS,
> ACPI_FORMAT_UINT64(acpi_gbl_xpm1a_status.address)));
>
> - lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
> + raw_spin_lock_irqsave(acpi_gbl_hardware_lock, lock_flags);
>
> /* Clear the fixed events in PM1 A/B */
>
> status = acpi_hw_register_write(ACPI_REGISTER_PM1_STATUS,
> ACPI_BITMASK_ALL_FIXED_STATUS);
>
> - acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
> + raw_spin_unlock_irqrestore(acpi_gbl_hardware_lock, lock_flags);
>
> if (ACPI_FAILURE(status))
> goto exit;
> Index: linux-stable/drivers/acpi/acpica/hwxface.c
> ===================================================================
> --- linux-stable.orig/drivers/acpi/acpica/hwxface.c
> +++ linux-stable/drivers/acpi/acpica/hwxface.c
> @@ -366,7 +366,7 @@ acpi_status acpi_write_bit_register(u32
> return_ACPI_STATUS(AE_BAD_PARAMETER);
> }
>
> - lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
> + raw_spin_lock_irqsave(acpi_gbl_hardware_lock, lock_flags);
>
> /*
> * At this point, we know that the parent register is one of the
> @@ -427,7 +427,7 @@ acpi_status acpi_write_bit_register(u32
>
> unlock_and_exit:
>
> - acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
> + raw_spin_unlock_irqrestore(acpi_gbl_hardware_lock, lock_flags);
> return_ACPI_STATUS(status);
> }
>
> Index: linux-stable/drivers/acpi/acpica/utmutex.c
> ===================================================================
> --- linux-stable.orig/drivers/acpi/acpica/utmutex.c
> +++ linux-stable/drivers/acpi/acpica/utmutex.c
> @@ -88,7 +88,7 @@ acpi_status acpi_ut_mutex_initialize(voi
> return_ACPI_STATUS (status);
> }
>
> - status = acpi_os_create_lock (&acpi_gbl_hardware_lock);
> + status = acpi_os_create_raw_lock (&acpi_gbl_hardware_lock);
> if (ACPI_FAILURE (status)) {
> return_ACPI_STATUS (status);
> }
> @@ -135,7 +135,7 @@ void acpi_ut_mutex_terminate(void)
> /* Delete the spinlocks */
>
> acpi_os_delete_lock(acpi_gbl_gpe_lock);
> - acpi_os_delete_lock(acpi_gbl_hardware_lock);
> + acpi_os_delete_raw_lock(acpi_gbl_hardware_lock);
>
> /* Delete the reader/writer lock */
>
> Index: linux-stable/include/acpi/platform/aclinux.h
> ===================================================================
> --- linux-stable.orig/include/acpi/platform/aclinux.h
> +++ linux-stable/include/acpi/platform/aclinux.h
> @@ -72,6 +72,7 @@
>
> #define acpi_cache_t struct kmem_cache
> #define acpi_spinlock spinlock_t *
> +#define acpi_raw_spinlock raw_spinlock_t *
> #define acpi_cpu_flags unsigned long
>
> #else /* !__KERNEL__ */
> @@ -175,6 +176,19 @@ static inline void *acpi_os_acquire_obje
> lock ? AE_OK : AE_NO_MEMORY; \
> })
>
> +#define acpi_os_create_raw_lock(__handle) \
> +({ \
> + raw_spinlock_t *lock = ACPI_ALLOCATE(sizeof(*lock)); \
> + \
> + if (lock) { \
> + *(__handle) = lock; \
> + raw_spin_lock_init(*(__handle)); \
> + } \
> + lock ? AE_OK : AE_NO_MEMORY; \
> +})
> +
> +#define acpi_os_delete_raw_lock(__handle) kfree(__handle)
> +
> #endif /* __KERNEL__ */
>
> #endif /* __ACLINUX_H__ */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
