Re: [PATCH 0/4] ACPI: hotplug messages improvement

Toshi Kani
2012-07-25T15:31:24Z

On Wed, 2012-07-25 at 12:45 +0900, Yasuaki Ishimatsu wrote:
> Hi Toshi,
>
> 2012/07/19 5:40, Toshi Kani wrote:
> > This patchset improves logging messages for ACPI CPU, Memory, and
> > Container hotplug notify handlers. The patchset introduces a set of
> > new macro interfaces, acpi_pr_<level>(), and updates the notify
> > handlers to use them. acpi_pr_<level>() appends "ACPI" prefix and
> > ACPI object path to the messages. This improves diagnostics in
> > hotplug operations since it identifies an object that caused an
> > issue in a log file.
> >
>
> The log message looks good to me.

Hi Yasuaki,

Thanks for reviewing!

> But I could not understand when to use it instead of pr_{warn, info, ...}
> or ACPI_{WARNING, INFO, ...}. Do you have the policy?

acpi_pr_<level>() is used when ACPI device path is used to identify an
ACPI object for the message, such as error message to the object. The
usage model is similar to dev_<level>(), which appends device object to
the message. acpi_pr_<level>() is intended for ACPI drivers, and can be
used when device object is not created / valid, such as the case for
ACPI hotplug handlers.

pr_<level>() is the regular printk() interfaces with message level, and
continues to be used when ACPI device path does not have to be appended
to the message. I expect non-error messages (such as boot-up messages)
continue to use pr_<level>().

ACPI_[WARNING|INFO|ERROR]() are ACPICA internal interfaces, and are not
intended for ACPI drivers.

Additionally, ACPI drivers can also use dev_<level>() when device object
is valid. You find such examples in patch 2/4.

I will add more descriptions to the patchset.

> > ---
> > This patchset applies on top of the patch below.
> >
> > [PATCH] ACPI: Add ACPI CPU hot-remove support
> > http://marc.info/?l=linux-acpi&m=134098193327362&w=2
> >
> > ---
> > Toshi Kani (4):
> > ACPI: Add acpi_pr_<level>() interfaces
> > ACPI: Update CPU hotplug messages
> > ACPI: Update Memory hotplug messages
> > ACPI: Update Container hotplug messages
>
> I think you need update other component, which are driver/acpi/{acpi_pad.c,
> battery.c, button.c}. Do you have the plan to update them?

I won't be ready to include them in this round, but will look at them
and change them later as necessary. I need to make sure that I can test
these drivers when making such changes.

Thanks,
-Toshi

>
> Thanks,
> Yasuaki Ishimatsu
>
> > ---
> > drivers/acpi/acpi_memhotplug.c | 24 ++++++++++++------------
> > drivers/acpi/container.c | 6 +++---
> > drivers/acpi/processor_driver.c | 36 +++++++++++++++++++++---------------
> > drivers/acpi/utils.c | 32 ++++++++++++++++++++++++++++++++
> > include/acpi/acpi_bus.h | 18 ++++++++++++++++++
> > 5 files changed, 86 insertions(+), 30 deletions(-)
> >
>
>