| Message ID | 1709720.Zl72FGBfpD@kreacher (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] xen: ACPI: Get rid of ACPICA message printing | expand |
On 2/24/21 1:47 PM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The ACPI_DEBUG_PRINT() macro is used in a few places in > xen-acpi-cpuhotplug.c and xen-acpi-memhotplug.c for printing debug > messages, but that is questionable, because that macro belongs to > ACPICA and it should not be used elsewhere. In addition, > ACPI_DEBUG_PRINT() requires special enabling to allow it to actually > print the message and the _COMPONENT symbol generally needed for > that is not defined in any of the files in question. > > For this reason, replace all of the ACPI_DEBUG_PRINT() instances in > the Xen code with acpi_handle_debug() (with the additional benefit > that the source object can be identified more easily after this > change) and drop the ACPI_MODULE_NAME() definitions that are only > used by the ACPICA message printing macros from that code. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
On 2/24/21 1:47 PM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The ACPI_DEBUG_PRINT() macro is used in a few places in > xen-acpi-cpuhotplug.c and xen-acpi-memhotplug.c for printing debug > messages, but that is questionable, because that macro belongs to > ACPICA and it should not be used elsewhere. In addition, > ACPI_DEBUG_PRINT() requires special enabling to allow it to actually > print the message and the _COMPONENT symbol generally needed for > that is not defined in any of the files in question. > > For this reason, replace all of the ACPI_DEBUG_PRINT() instances in > the Xen code with acpi_handle_debug() (with the additional benefit > that the source object can be identified more easily after this > change) and drop the ACPI_MODULE_NAME() definitions that are only > used by the ACPICA message printing macros from that code. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/xen/xen-acpi-cpuhotplug.c | 12 +++++------- > drivers/xen/xen-acpi-memhotplug.c | 16 +++++++--------- As I was building with this patch I (re-)discovered that since 2013 it depends on BROKEN (commit 76fc253723add). Despite commit message there saying that it's a temporary patch it seems to me by now that it's more than that. And clearly noone tried to build these files since at least 2015 because memhotplug file doesn't compile due to commit cfafae940381207. While this is easily fixable the question is whether we want to keep these files. Obviously noone cares about this functionality. -boris
On Sun, Feb 28, 2021 at 2:49 AM Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote: > > > On 2/24/21 1:47 PM, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > The ACPI_DEBUG_PRINT() macro is used in a few places in > > xen-acpi-cpuhotplug.c and xen-acpi-memhotplug.c for printing debug > > messages, but that is questionable, because that macro belongs to > > ACPICA and it should not be used elsewhere. In addition, > > ACPI_DEBUG_PRINT() requires special enabling to allow it to actually > > print the message and the _COMPONENT symbol generally needed for > > that is not defined in any of the files in question. > > > > For this reason, replace all of the ACPI_DEBUG_PRINT() instances in > > the Xen code with acpi_handle_debug() (with the additional benefit > > that the source object can be identified more easily after this > > change) and drop the ACPI_MODULE_NAME() definitions that are only > > used by the ACPICA message printing macros from that code. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/xen/xen-acpi-cpuhotplug.c | 12 +++++------- > > drivers/xen/xen-acpi-memhotplug.c | 16 +++++++--------- > > > As I was building with this patch I (re-)discovered that since 2013 it depends on BROKEN (commit 76fc253723add). Despite commit message there saying that it's a temporary patch it seems to me by now that it's more than that. > > > And clearly noone tried to build these files since at least 2015 because memhotplug file doesn't compile due to commit cfafae940381207. > > > While this is easily fixable the question is whether we want to keep these files. Obviously noone cares about this functionality. Well, I would be for dropping them. Do you want me to send a patch to do that?
On 3/1/21 9:11 AM, Rafael J. Wysocki wrote: > On Sun, Feb 28, 2021 at 2:49 AM Boris Ostrovsky > <boris.ostrovsky@oracle.com> wrote: >> >> On 2/24/21 1:47 PM, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> The ACPI_DEBUG_PRINT() macro is used in a few places in >>> xen-acpi-cpuhotplug.c and xen-acpi-memhotplug.c for printing debug >>> messages, but that is questionable, because that macro belongs to >>> ACPICA and it should not be used elsewhere. In addition, >>> ACPI_DEBUG_PRINT() requires special enabling to allow it to actually >>> print the message and the _COMPONENT symbol generally needed for >>> that is not defined in any of the files in question. >>> >>> For this reason, replace all of the ACPI_DEBUG_PRINT() instances in >>> the Xen code with acpi_handle_debug() (with the additional benefit >>> that the source object can be identified more easily after this >>> change) and drop the ACPI_MODULE_NAME() definitions that are only >>> used by the ACPICA message printing macros from that code. >>> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> --- >>> drivers/xen/xen-acpi-cpuhotplug.c | 12 +++++------- >>> drivers/xen/xen-acpi-memhotplug.c | 16 +++++++--------- >> >> As I was building with this patch I (re-)discovered that since 2013 it depends on BROKEN (commit 76fc253723add). Despite commit message there saying that it's a temporary patch it seems to me by now that it's more than that. >> >> >> And clearly noone tried to build these files since at least 2015 because memhotplug file doesn't compile due to commit cfafae940381207. >> >> >> While this is easily fixable the question is whether we want to keep these files. Obviously noone cares about this functionality. > Well, I would be for dropping them. > > Do you want me to send a patch to do that? Yes, if you don't mind (but let's give this a few days for people to have a chance to comment). -boris
On 01.03.21 17:16, Boris Ostrovsky wrote: > > On 3/1/21 9:11 AM, Rafael J. Wysocki wrote: >> On Sun, Feb 28, 2021 at 2:49 AM Boris Ostrovsky >> <boris.ostrovsky@oracle.com> wrote: >>> >>> On 2/24/21 1:47 PM, Rafael J. Wysocki wrote: >>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>>> >>>> The ACPI_DEBUG_PRINT() macro is used in a few places in >>>> xen-acpi-cpuhotplug.c and xen-acpi-memhotplug.c for printing debug >>>> messages, but that is questionable, because that macro belongs to >>>> ACPICA and it should not be used elsewhere. In addition, >>>> ACPI_DEBUG_PRINT() requires special enabling to allow it to actually >>>> print the message and the _COMPONENT symbol generally needed for >>>> that is not defined in any of the files in question. >>>> >>>> For this reason, replace all of the ACPI_DEBUG_PRINT() instances in >>>> the Xen code with acpi_handle_debug() (with the additional benefit >>>> that the source object can be identified more easily after this >>>> change) and drop the ACPI_MODULE_NAME() definitions that are only >>>> used by the ACPICA message printing macros from that code. >>>> >>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>>> --- >>>> drivers/xen/xen-acpi-cpuhotplug.c | 12 +++++------- >>>> drivers/xen/xen-acpi-memhotplug.c | 16 +++++++--------- >>> >>> As I was building with this patch I (re-)discovered that since 2013 it depends on BROKEN (commit 76fc253723add). Despite commit message there saying that it's a temporary patch it seems to me by now that it's more than that. >>> >>> >>> And clearly noone tried to build these files since at least 2015 because memhotplug file doesn't compile due to commit cfafae940381207. >>> >>> >>> While this is easily fixable the question is whether we want to keep these files. Obviously noone cares about this functionality. >> Well, I would be for dropping them. >> >> Do you want me to send a patch to do that? > > > Yes, if you don't mind (but let's give this a few days for people to have a chance to comment). I'm fine with removing those files. Juergen
Index: linux-pm/drivers/xen/xen-acpi-cpuhotplug.c =================================================================== --- linux-pm.orig/drivers/xen/xen-acpi-cpuhotplug.c +++ linux-pm/drivers/xen/xen-acpi-cpuhotplug.c @@ -242,10 +242,10 @@ static void acpi_processor_hotplug_notif switch (event) { case ACPI_NOTIFY_BUS_CHECK: case ACPI_NOTIFY_DEVICE_CHECK: - ACPI_DEBUG_PRINT((ACPI_DB_INFO, + acpi_handle_debug(handle, "Processor driver received %s event\n", (event == ACPI_NOTIFY_BUS_CHECK) ? - "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK")); + "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK"); if (!is_processor_present(handle)) break; @@ -269,8 +269,8 @@ static void acpi_processor_hotplug_notif break; case ACPI_NOTIFY_EJECT_REQUEST: - ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "received ACPI_NOTIFY_EJECT_REQUEST\n")); + acpi_handle_debug(handle, + "received ACPI_NOTIFY_EJECT_REQUEST\n"); if (acpi_bus_get_device(handle, &device)) { pr_err(PREFIX "Device don't exist, dropping EJECT\n"); @@ -290,8 +290,7 @@ static void acpi_processor_hotplug_notif break; default: - ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "Unsupported event [0x%x]\n", event)); + acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event); /* non-hotplug event; possibly handled by other handler */ goto out; @@ -440,7 +439,6 @@ static void __exit xen_acpi_processor_ex module_init(xen_acpi_processor_init); module_exit(xen_acpi_processor_exit); -ACPI_MODULE_NAME("xen-acpi-cpuhotplug"); MODULE_AUTHOR("Liu Jinsong <jinsong.liu@intel.com>"); MODULE_DESCRIPTION("Xen Hotplug CPU Driver"); MODULE_LICENSE("GPL"); Index: linux-pm/drivers/xen/xen-acpi-memhotplug.c =================================================================== --- linux-pm.orig/drivers/xen/xen-acpi-memhotplug.c +++ linux-pm/drivers/xen/xen-acpi-memhotplug.c @@ -227,13 +227,13 @@ static void acpi_memory_device_notify(ac switch (event) { case ACPI_NOTIFY_BUS_CHECK: - ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "\nReceived BUS CHECK notification for device\n")); + acpi_handle_debug(handle, + "Received BUS CHECK notification for device\n"); fallthrough; case ACPI_NOTIFY_DEVICE_CHECK: if (event == ACPI_NOTIFY_DEVICE_CHECK) - ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "\nReceived DEVICE CHECK notification for device\n")); + acpi_handle_debug(handle, + "Received DEVICE CHECK notification for device\n"); if (acpi_memory_get_device(handle, &mem_device)) { pr_err(PREFIX "Cannot find driver data\n"); @@ -244,8 +244,8 @@ static void acpi_memory_device_notify(ac break; case ACPI_NOTIFY_EJECT_REQUEST: - ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "\nReceived EJECT REQUEST notification for device\n")); + acpi_handle_debug(handle, + "Received EJECT REQUEST notification for device\n"); acpi_scan_lock_acquire(); if (acpi_bus_get_device(handle, &device)) { @@ -269,8 +269,7 @@ static void acpi_memory_device_notify(ac break; default: - ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "Unsupported event [0x%x]\n", event)); + acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event); /* non-hotplug event; possibly handled by other handler */ return; } @@ -469,7 +468,6 @@ static void __exit xen_acpi_memory_devic module_init(xen_acpi_memory_device_init); module_exit(xen_acpi_memory_device_exit); -ACPI_MODULE_NAME("xen-acpi-memhotplug"); MODULE_AUTHOR("Liu Jinsong <jinsong.liu@intel.com>"); MODULE_DESCRIPTION("Xen Hotplug Mem Driver"); MODULE_LICENSE("GPL");