Re: [PATCH v1] drivers/acpi/scan.c: Fixup "acquire device_hotplug_lock in acpi_scan_init()"

[Michal Hocko <mhocko@kernel.org>]

On Wed 31-07-19 14:32:01, David Hildenbrand wrote:
> Let's document why we take the lock here. If we're going to overhaul
> memory hotplug locking, we'll have to touch many places - this comment
> will help to clairfy why it was added here.

And how exactly is "lock for consistency" comment going to help the poor
soul touching that code? How do people know that it is safe to remove it?
I am not going to repeat my arguments how/why I hate "locking for
consistency" (or fun or whatever but a real synchronization reasons)
but if you want to help then just explicitly state what should done to
remove this lock.

> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  drivers/acpi/scan.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index cbc9d64b48dd..9193f1d46148 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2204,6 +2204,11 @@ int __init acpi_scan_init(void)
>  	acpi_gpe_apply_masked_gpes();
>  	acpi_update_all_gpes();
>  
> +	/*
> +	 * We end up calling __add_memory(), which expects the
> +	 * device_hotplug_lock to be held. Races with userspace and other
> +	 * hotplug activities are not really possible - lock for consistency.
> +	 */
>  	lock_device_hotplug();
>  	mutex_lock(&acpi_scan_lock);
>  
> -- 
> 2.21.0

-- 
Michal Hocko
SUSE Labs