From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4DA15C432C0 for ; Sun, 1 Dec 2019 01:54:17 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 01D4E215F1 for ; Sun, 1 Dec 2019 01:54:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="deXkOS6n" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 01D4E215F1 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A3E726B02FB; Sat, 30 Nov 2019 20:54:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9F2C26B02FD; Sat, 30 Nov 2019 20:54:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9044A6B02FE; Sat, 30 Nov 2019 20:54:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0164.hostedemail.com [216.40.44.164]) by kanga.kvack.org (Postfix) with ESMTP id 7AF496B02FB for ; Sat, 30 Nov 2019 20:54:16 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with SMTP id 2A1DC180AD817 for ; Sun, 1 Dec 2019 01:54:16 +0000 (UTC) X-FDA: 76214902512.23.bean16_2539023dcb24e X-HE-Tag: bean16_2539023dcb24e X-Filterd-Recvd-Size: 7449 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf24.hostedemail.com (Postfix) with ESMTP for ; Sun, 1 Dec 2019 01:54:15 +0000 (UTC) Received: from localhost.localdomain (c-73-231-172-41.hsd1.ca.comcast.net [73.231.172.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B9992215E5; Sun, 1 Dec 2019 01:54:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1575165255; bh=elwkYVOk8NfEg8EycBSx9jvMs2CPaiHvyOt5dSoucKc=; h=Date:From:To:Subject:From; b=deXkOS6n1KzOGCd87BzZYm5CtvSmddA+UzkAp4zES/D/tvQbn5jSKMHBcQmPgo/lB chRfi/d8NfJsuCLqkKPFO0uHyWNycfemhYbyFcNN3nQg/sHr3+SJBu0xBsgkD5JPPB Y1SQcyKeOHPuhtZrMW96iCN1sIVo/0hzR/KmCUqI= Date: Sat, 30 Nov 2019 17:54:14 -0800 From: akpm@linux-foundation.org To: akpm@linux-foundation.org, david@redhat.com, gregkh@linuxfoundation.org, linux-mm@kvack.org, mhocko@kernel.org, mm-commits@vger.kernel.org, osalvador@suse.de, rafael@kernel.org, torvalds@linux-foundation.org Subject: [patch 082/158] drivers/base/memory.c: drop the mem_sysfs_mutex Message-ID: <20191201015414.1DORDFq_C%akpm@linux-foundation.org> User-Agent: s-nail v14.8.16 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: From: David Hildenbrand Subject: drivers/base/memory.c: drop the mem_sysfs_mutex The mem_sysfs_mutex isn't really helpful. Also, it's not really clear what the mutex protects at all. The device lists of the memory subsystem are protected separately. We don't need that mutex when looking up. creating, or removing independent devices. find_memory_block_by_id() will perform locking on its own and grab a reference of the returned device. At the time memory_dev_init() is called, we cannot have concurrent hot(un)plug operations yet - we're still fairly early during boot. We don't need any locking. The creation/removal of memory block devices should be protected on a higher level - especially using the device hotplug lock to avoid documented issues (see Documentation/core-api/memory-hotplug.rst) - or if that is reworked, using similar locking. Protecting in the context of these functions only doesn't really make sense. Especially, if we would have a situation where the same memory blocks are created/deleted at the same time, there is something horribly going wrong (imagining adding/removing a DIMM at the same time from two call paths) - after the functions succeeded something else in the callers would blow up (e.g., create_memory_block_devices() succeeded but there are no memory block devices anymore). All relevant call paths (except when adding memory early during boot via ACPI, which is now documented) hold the device hotplug lock when adding memory, and when removing memory. Let's document that instead. Add a simple safety net to create_memory_block_devices() in case we would actually remove memory blocks while adding them, so we'll never dereference a NULL pointer. Simplify memory_dev_init() now that the lock is gone. Link: http://lkml.kernel.org/r/20190925082621.4927-1-david@redhat.com Signed-off-by: David Hildenbrand Reviewed-by: Andrew Morton Cc: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" Cc: Michal Hocko Cc: Oscar Salvador Signed-off-by: Andrew Morton --- drivers/base/memory.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) --- a/drivers/base/memory.c~drivers-base-memoryc-drop-the-mem_sysfs_mutex +++ a/drivers/base/memory.c @@ -19,15 +19,12 @@ #include #include #include -#include #include #include #include #include -static DEFINE_MUTEX(mem_sysfs_mutex); - #define MEMORY_CLASS_NAME "memory" #define to_memory_block(dev) container_of(dev, struct memory_block, dev) @@ -700,6 +697,8 @@ static void unregister_memory(struct mem * Create memory block devices for the given memory area. Start and size * have to be aligned to memory block granularity. Memory block devices * will be initialized as offline. + * + * Called under device_hotplug_lock. */ int create_memory_block_devices(unsigned long start, unsigned long size) { @@ -713,7 +712,6 @@ int create_memory_block_devices(unsigned !IS_ALIGNED(size, memory_block_size_bytes()))) return -EINVAL; - mutex_lock(&mem_sysfs_mutex); for (block_id = start_block_id; block_id != end_block_id; block_id++) { ret = init_memory_block(&mem, block_id, MEM_OFFLINE); if (ret) @@ -725,11 +723,12 @@ int create_memory_block_devices(unsigned for (block_id = start_block_id; block_id != end_block_id; block_id++) { mem = find_memory_block_by_id(block_id); + if (WARN_ON_ONCE(!mem)) + continue; mem->section_count = 0; unregister_memory(mem); } } - mutex_unlock(&mem_sysfs_mutex); return ret; } @@ -737,6 +736,8 @@ int create_memory_block_devices(unsigned * Remove memory block devices for the given memory area. Start and size * have to be aligned to memory block granularity. Memory block devices * have to be offline. + * + * Called under device_hotplug_lock. */ void remove_memory_block_devices(unsigned long start, unsigned long size) { @@ -749,7 +750,6 @@ void remove_memory_block_devices(unsigne !IS_ALIGNED(size, memory_block_size_bytes()))) return; - mutex_lock(&mem_sysfs_mutex); for (block_id = start_block_id; block_id != end_block_id; block_id++) { mem = find_memory_block_by_id(block_id); if (WARN_ON_ONCE(!mem)) @@ -758,7 +758,6 @@ void remove_memory_block_devices(unsigne unregister_memory_block_under_nodes(mem); unregister_memory(mem); } - mutex_unlock(&mem_sysfs_mutex); } /* return true if the memory block is offlined, otherwise, return false */ @@ -792,12 +791,13 @@ static const struct attribute_group *mem }; /* - * Initialize the sysfs support for memory devices... + * Initialize the sysfs support for memory devices. At the time this function + * is called, we cannot have concurrent creation/deletion of memory block + * devices, the device_hotplug_lock is not needed. */ void __init memory_dev_init(void) { int ret; - int err; unsigned long block_sz, nr; /* Validate the configured memory block size */ @@ -808,24 +808,19 @@ void __init memory_dev_init(void) ret = subsys_system_register(&memory_subsys, memory_root_attr_groups); if (ret) - goto out; + panic("%s() failed to register subsystem: %d\n", __func__, ret); /* * Create entries for memory sections that were found * during boot and have been initialized */ - mutex_lock(&mem_sysfs_mutex); for (nr = 0; nr <= __highest_present_section_nr; nr += sections_per_block) { - err = add_memory_block(nr); - if (!ret) - ret = err; + ret = add_memory_block(nr); + if (ret) + panic("%s() failed to add memory block: %d\n", __func__, + ret); } - mutex_unlock(&mem_sysfs_mutex); - -out: - if (ret) - panic("%s() failed: %d\n", __func__, ret); } /** _