13 Aug, 2013

1 commit

  • CPU system maps are protected with reader/writer locks. The reader
    lock, get_online_cpus(), assures that the maps are not updated while
    holding the lock. The writer lock, cpu_hotplug_begin(), is used to
    udpate the cpu maps along with cpu_maps_update_begin().

    However, the ACPI processor handler updates the cpu maps without
    holding the the writer lock.

    acpi_map_lsapic() is called from acpi_processor_hotadd_init() to
    update cpu_possible_mask and cpu_present_mask. acpi_unmap_lsapic()
    is called from acpi_processor_remove() to update cpu_possible_mask.
    Currently, they are either unprotected or protected with the reader
    lock, which is not correct.

    For example, the get_online_cpus() below is supposed to assure that
    cpu_possible_mask is not changed while the code is iterating with
    for_each_possible_cpu().

    get_online_cpus();
    for_each_possible_cpu(cpu) {
    :
    }
    put_online_cpus();

    However, this lock has no protection with CPU hotplug since the ACPI
    processor handler does not use the writer lock when it updates
    cpu_possible_mask. The reader lock does not serialize within the
    readers.

    This patch protects them with the writer lock with cpu_hotplug_begin()
    along with cpu_maps_update_begin(), which must be held before calling
    cpu_hotplug_begin(). It also protects arch_register_cpu() /
    arch_unregister_cpu(), which creates / deletes a sysfs cpu device
    interface. For this purpose it changes cpu_hotplug_begin() and
    cpu_hotplug_done() to global and exports them in cpu.h.

    Signed-off-by: Toshi Kani
    Signed-off-by: Rafael J. Wysocki

    Toshi Kani
     

08 Aug, 2013

1 commit

  • try_offline_node() checks that all CPUs associated with the given
    node have been removed by using cpu_present_bits. If all cpus
    related to that node have been removed, try_offline_node() clears
    the node information.

    However, try_offline_node() called from acpi_processor_remove() never
    clears the node information. For disabling cpu_present_bits,
    acpi_unmap_lsapic() needs be called. Yet, acpi_unmap_lsapic() is
    called after try_offline_node() has run. So when try_offline_node()
    runs, the CPU's cpu_present_bits is always set.

    Fix the issue by moving try_offline_node() after acpi_unmap_lsapic().

    The problem fixed here was uncovered by commit cecdb19 "ACPI / scan:
    Change the implementation of acpi_bus_trim()".

    [rjw: Changelog]
    Signed-off-by: Yasuaki Ishimatsu
    Acked-by: Toshi Kani
    Cc: 3.9+ # 3.9+
    Signed-off-by: Rafael J. Wysocki

    Yasuaki Ishimatsu
     

15 Jul, 2013

1 commit

  • The __cpuinit type of throwaway sections might have made sense
    some time ago when RAM was more constrained, but now the savings
    do not offset the cost and complications. For example, the fix in
    commit 5e427ec2d0 ("x86: Fix bit corruption at CPU resume time")
    is a good example of the nasty type of bugs that can be created
    with improper use of the various __init prefixes.

    After a discussion on LKML[1] it was decided that cpuinit should go
    the way of devinit and be phased out. Once all the users are gone,
    we can then finally remove the macros themselves from linux/init.h.

    This removes all the drivers/acpi uses of the __cpuinit macros
    from all C files.

    [1] https://lkml.org/lkml/2013/5/20/589

    Cc: Len Brown
    Cc: "Rafael J. Wysocki"
    Cc: linux-acpi@vger.kernel.org
    Signed-off-by: Paul Gortmaker

    Paul Gortmaker
     

02 Jun, 2013

2 commits


31 May, 2013

1 commit

  • Commit ac212b6 (ACPI / processor: Use common hotplug infrastructure)
    forgot about initializing the per-CPU 'processors' variables which
    lead to ACPI cpuidle failure to use C-states and caused boot slowdown
    on multi-CPU machines.

    Fix the problem by adding per_cpu(processors, pr->id) initialization
    to acpi_processor_add() and add make acpi_processor_remove() clean it
    up as appropriate.

    Also modify acpi_processor_stop() so that it doesn't clear
    per_cpu(processors, pr->id) on processor driver removal which would
    then cause problems to happen when the driver is loaded again.

    This version of the patch contains fixes from Yinghai Lu.

    Reported-and-tested-by: Yinghai Lu
    Reported-and-tested-by: Daniel Lezcano
    Signed-off-by: Rafael J. Wysocki

    Rafael J. Wysocki
     

12 May, 2013

1 commit

  • Split the ACPI processor driver into two parts, one that is
    non-modular, resides in the ACPI core and handles the enumeration
    and hotplug of processors and one that implements the rest of the
    existing processor driver functionality.

    The non-modular part uses an ACPI scan handler object to enumerate
    processors on the basis of information provided by the ACPI namespace
    and to hook up with the common ACPI hotplug infrastructure. It also
    populates the ACPI handle of each processor device having a
    corresponding object in the ACPI namespace, which allows the driver
    proper to bind to those devices, and makes the driver bind to them
    if it is readily available (i.e. loaded) when the scan handler's
    .attach() routine is running.

    There are a few reasons to make this change.

    First, switching the ACPI processor driver to using the common ACPI
    hotplug infrastructure reduces code duplication and size considerably,
    even though a new file is created along with a header comment etc.

    Second, since the common hotplug code attempts to offline devices
    before starting the (non-reversible) removal procedure, it will abort
    (and possibly roll back) hot-remove operations involving processors
    if cpu_down() returns an error code for one of them instead of
    continuing them blindly (if /sys/firmware/acpi/hotplug/force_remove
    is unset). That is a more desirable behavior than what the current
    code does.

    Finally, the separation of the scan/hotplug part from the driver
    proper makes it possible to simplify the driver's .remove() routine,
    because it doesn't need to worry about the possible cleanup related
    to processor removal any more (the scan/hotplug part is responsible
    for that now) and can handle device removal and driver removal
    symmetricaly (i.e. as appropriate).

    Some user-visible changes in sysfs are made (for example, the
    'sysdev' link from the ACPI device node to the processor device's
    directory is gone and a 'physical_node' link is present instead
    and a corresponding 'firmware_node' is present in the processor
    device's directory, the processor driver is now visible under
    /sys/bus/cpu/drivers/ and bound to the processor device), but
    that shouldn't affect the functionality that users care about
    (frequency scaling, C-states and thermal management).

    Tested on my venerable Toshiba Portege R500.

    Signed-off-by: Rafael J. Wysocki
    Acked-by: Greg Kroah-Hartman
    Reviewed-by: Toshi Kani

    Rafael J. Wysocki