lkml.org 
[lkml]   [2013]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: [Update 3][PATCH 2/7] ACPI / scan: Introduce common code for ACPI-based device hotplug
2013/02/22 10:50, Rafael J. Wysocki wrote:
> On Thursday, February 21, 2013 06:12:21 PM Toshi Kani wrote:
>> On Fri, 2013-02-22 at 00:06 +0100, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> Multiple drivers handling hotplug-capable ACPI device nodes install
>>> notify handlers covering the same types of events in a very similar
>>> way. Moreover, those handlers are installed in separate namespace
>>> walks, although that really should be done during namespace scans
>>> carried out by acpi_bus_scan(). This leads to substantial code
>>> duplication, unnecessary overhead and behavior that is hard to
>>> follow.
>>>
>>> For this reason, introduce common code in drivers/acpi/scan.c for
>>> handling hotplug-related notification and carrying out device
>>> insertion and eject operations in a generic fashion, such that it
>>> may be used by all of the relevant drivers in the future. To cover
>>> the existing differences between those drivers introduce struct
>>> acpi_hotplug_profile for representing collections of hotplug
>>> settings associated with different ACPI scan handlers that can be
>>> used by the drivers to make the common code reflect their current
>>> behavior.
>>>
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> ---
>>>
>>> This update fixes an issue pointed out by Toshi Kani (that
>>> ACPI_OST_EC_OSPM_* event source codes should not be used with _OST for events
>>> that we received a notification for from the platform firmware).
>>>
>>> Thanks,
>>> Rafael
>>>
>>> ---
>>> drivers/acpi/scan.c | 270 ++++++++++++++++++++++++++++++++++++++----------
>>> include/acpi/acpi_bus.h | 7 +
>>> 2 files changed, 224 insertions(+), 53 deletions(-)
>> :
>>> +static void acpi_bus_device_eject(void *context)
>>> +{
>>> + acpi_handle handle = context;
>>> + struct acpi_device *device = NULL;
>>> + struct acpi_scan_handler *handler;
>>> + u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
>>> +
>>> + mutex_lock(&acpi_scan_lock);
>>> +
>>> + acpi_bus_get_device(handle, &device);
>>> + if (!device)
>>> + goto err_out;
>>> +
>>> + handler = device->handler;
>>> + if (!handler || !handler->hotplug.enabled) {
>>> + ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
>>> + goto err_out;
>>> + }
>>> + acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
>>> + ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
>>> + if (handler->hotplug.autoeject) {
>>> + int error;
>>> +
>>> + get_device(&device->dev);
>>> + error = acpi_scan_hot_remove(device);
>>> + if (error)
>>> + goto err_out;
>>> + } else {
>>> + device->flags.eject_pending = true;
>>> }
>>> + if (handler->hotplug.uevents)
>>> + kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
>>
>> I confirmed that the _OST issue with hot-add is fixed. Here is a new
>> one. When autoeject is enabled, it crashes in kobject_uevent() since
>> the device is no longer valid.
>
> Well, this one is more difficult.
>
> I can change the ordering so that kobject_uevent() is called before
> acpi_scan_hot_remove(), but then user space may not know that the device is
> being removed at the moment (which still may fail). Still, maybe this is
> OK, because user space will get KOBJ_REMOVE when the device actually goes
> away anyway.
>
> Or perhaps we can emit KOBJ_OFFLINE before acpi_scan_hot_remove() and if
> it fails, emit KOBJ_ONLINE?

How about following patch? My system cannot send EJECT notification.
So I have not tested this patch.

# Recently when I send a patch, tabs of the patch is changed to spaces often.
# So I attached the patch.

---

When hotplug.autoeject and uevents are enabled, it crashes in
kobject_uevent() since the device is no longer valid.

This patch fixes the problem.

Reported-by: Toshi Kani <toshi.kani@hp.com>
Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

---
drivers/acpi/scan.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c 2013-02-22 16:02:07.000000000 +0900
+++ linux-pm/drivers/acpi/scan.c 2013-02-22 16:06:36.792816699 +0900
@@ -139,9 +139,6 @@ static int acpi_scan_hot_remove(struct a
"Hot-removing device %s...\n", dev_name(&device->dev)));

acpi_bus_trim(device);
- /* Device node has been unregistered. */
- put_device(&device->dev);
- device = NULL;

if (ACPI_SUCCESS(acpi_get_handle(handle, "_LCK", &not_used))) {
arg_list.count = 1;
@@ -191,10 +188,10 @@ static void acpi_bus_device_eject(void *
}
acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
+ get_device(&device->dev);
if (handler->hotplug.autoeject) {
int error;

- get_device(&device->dev);
error = acpi_scan_hot_remove(device);
if (error)
goto err_out;
@@ -204,6 +201,7 @@ static void acpi_bus_device_eject(void *
if (handler->hotplug.uevents)
kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);

+ put_device(&device->dev);
out:
mutex_unlock(&acpi_scan_lock);
return;
@@ -312,6 +310,7 @@ void acpi_bus_hot_remove_device(void *co
ACPI_OST_SC_NON_SPECIFIC_FAILURE,
NULL);

+ put_device(&acpi_device->dev);
mutex_unlock(&acpi_scan_lock);
kfree(context);
}
When hotplug.autoeject and uevents are enabled, it crashes in
kobject_uevent() since the device is no longer valid.

This patch fixes the problem.

Reported-by: Toshi Kani <toshi.kani@hp.com>
Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

---
drivers/acpi/scan.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c 2013-02-22 16:02:07.000000000 +0900
+++ linux-pm/drivers/acpi/scan.c 2013-02-22 16:06:36.792816699 +0900
@@ -139,9 +139,6 @@ static int acpi_scan_hot_remove(struct a
"Hot-removing device %s...\n", dev_name(&device->dev)));

acpi_bus_trim(device);
- /* Device node has been unregistered. */
- put_device(&device->dev);
- device = NULL;

if (ACPI_SUCCESS(acpi_get_handle(handle, "_LCK", &not_used))) {
arg_list.count = 1;
@@ -191,10 +188,10 @@ static void acpi_bus_device_eject(void *
}
acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
+ get_device(&device->dev);
if (handler->hotplug.autoeject) {
int error;

- get_device(&device->dev);
error = acpi_scan_hot_remove(device);
if (error)
goto err_out;
@@ -204,6 +201,7 @@ static void acpi_bus_device_eject(void *
if (handler->hotplug.uevents)
kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);

+ put_device(&device->dev);
out:
mutex_unlock(&acpi_scan_lock);
return;
@@ -312,6 +310,7 @@ void acpi_bus_hot_remove_device(void *co
ACPI_OST_SC_NON_SPECIFIC_FAILURE,
NULL);

+ put_device(&acpi_device->dev);
mutex_unlock(&acpi_scan_lock);
kfree(context);
}
\
 
 \ /
  Last update: 2013-02-22 10:21    [W:0.145 / U:0.908 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site