From patchwork Tue Feb 11 17:43:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Henrique Cerri X-Patchwork-Id: 1236402 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48H9BS2ZtNz9sRX; Wed, 12 Feb 2020 04:44:13 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1j1ZZx-00054o-KX; Tue, 11 Feb 2020 17:44:09 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1j1ZZv-00054V-Ip for kernel-team@lists.ubuntu.com; Tue, 11 Feb 2020 17:44:07 +0000 Received: from mail-qt1-f200.google.com ([209.85.160.200]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1j1ZZv-0003cb-8C for kernel-team@lists.ubuntu.com; Tue, 11 Feb 2020 17:44:07 +0000 Received: by mail-qt1-f200.google.com with SMTP id r9so7100390qtc.4 for ; Tue, 11 Feb 2020 09:44:07 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=pzaLfDQPcKbyN0EYCfeSH9Yo8AtjLYp2N+HcTn3mQuQ=; b=EuQASsY2i7uzbRFNK6cwFDYCFneVlK2vPXFDn1zjm6aoi6Urd96X1qvTyrY4ET7Tgi TfuPj5064/Q60pK8RA6vGY5Zg5DrrnGU84pqyiaN/VvKis86Ig1R/MTy9XZvQ1gqAR9x RmIOu7xwmYJwsBBHoiYcgbKaY49Xjd3Vf7T5oHtUFa260O4u7p3P1nh70qOaSCcy0SOG +m2Cicjf1WJEHoTDRC+BCK3WwwB6tawqGX6JEUdJ56qxI/CQP9bB9PEvD5OlQt1FKOdq AvYfRWdnxDjvK/FXtz66UXPD/TupqLDgkqX5s5+EoDE0VydJgOIMqiy5XbIkx093vjTg 6BEA== X-Gm-Message-State: APjAAAWwIbyE3Gywd1gxHo3zPm55Wb/tx8S/4wSnSvlDFiyJMCsn2Hny QVsOBQfuU46FkmvugENWfmcF5YjBiW/nAsv4jfLoK+Uckk7G5bg+epwuTZKnh79wwemHAUZTRbw u4ATW/Ms5N6tiAhSXpho9TzdEj0EDfyw4YGreZqm9 X-Received: by 2002:ad4:4d94:: with SMTP id cv20mr3709761qvb.99.1581443045869; Tue, 11 Feb 2020 09:44:05 -0800 (PST) X-Google-Smtp-Source: APXvYqx4PtvLxVmlYQuwk1MEv6efYLsaSQrTt3GFqS/6QtgbscD9WjWftac/XOr0RXw6Hxp5tMs3Hg== X-Received: by 2002:ad4:4d94:: with SMTP id cv20mr3709735qvb.99.1581443045520; Tue, 11 Feb 2020 09:44:05 -0800 (PST) Received: from gallifrey.lan ([2804:14c:4e6:34d:c976:c9a5:3dcb:863]) by smtp.gmail.com with ESMTPSA id w9sm2343433qka.71.2020.02.11.09.44.03 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Feb 2020 09:44:04 -0800 (PST) From: Marcelo Henrique Cerri To: kernel-team@lists.ubuntu.com Subject: [bionic:linux][PATCH 1/2] Revert "mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock" Date: Tue, 11 Feb 2020 14:43:58 -0300 Message-Id: <20200211174359.12230-2-marcelo.cerri@canonical.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200211174359.12230-1-marcelo.cerri@canonical.com> References: <20200211174359.12230-1-marcelo.cerri@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" BugLink: https://bugs.launchpad.net/bugs/1862312 This reverts commit 483735440add2530c03da656ede83ee81414a566. The backport for upstream commit 381eab4a6ee8 (mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock) placed the call to mem_hotplug_begin() on the wrong location and caused a regression. Signed-off-by: Marcelo Henrique Cerri --- drivers/base/memory.c | 13 ++++++++++++- mm/memory_hotplug.c | 28 +++++++--------------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index a3b8aebb95cb..fe1557aa9b10 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -224,6 +224,7 @@ static bool pages_correctly_reserved(unsigned long start_pfn) /* * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is * OK to have direct references to sparsemem variables in here. + * Must already be protected by mem_hotplug_begin(). */ static int memory_block_action(unsigned long phys_index, unsigned long action, int online_type) @@ -289,6 +290,7 @@ static int memory_subsys_online(struct device *dev) if (mem->online_type < 0) mem->online_type = MMOP_ONLINE_KEEP; + /* Already under protection of mem_hotplug_begin() */ ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE); /* clear online_type */ @@ -335,11 +337,19 @@ store_mem_state(struct device *dev, goto err; } + /* + * Memory hotplug needs to hold mem_hotplug_begin() for probe to find + * the correct memory block to online before doing device_online(dev), + * which will take dev->mutex. Take the lock early to prevent an + * inversion, memory_subsys_online() callbacks will be implemented by + * assuming it's already protected. + */ + mem_hotplug_begin(); + switch (online_type) { case MMOP_ONLINE_KERNEL: case MMOP_ONLINE_MOVABLE: case MMOP_ONLINE_KEEP: - /* mem->online_type is protected by device_hotplug_lock */ mem->online_type = online_type; ret = device_online(&mem->dev); break; @@ -350,6 +360,7 @@ store_mem_state(struct device *dev, ret = -EINVAL; /* should never happen */ } + mem_hotplug_done(); err: unlock_device_hotplug(); diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 6cd0c4a144d5..6f9e895ce21a 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -881,6 +881,7 @@ static struct zone * __meminit move_pfn_range(int online_type, int nid, return zone; } +/* Must be protected by mem_hotplug_begin() or a device_lock */ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_type) { unsigned long flags; @@ -904,8 +905,6 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ if (ret) goto failed_addition; - mem_hotplug_begin(); - /* * If this zone is not populated, then it is not in zonelist. * This means the page allocator ignores this zone. @@ -951,7 +950,6 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ if (onlined_pages) memory_notify(MEM_ONLINE, &arg); - mem_hotplug_done(); return 0; failed_addition: @@ -959,7 +957,6 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ (unsigned long long) pfn << PAGE_SHIFT, (((unsigned long long) pfn + nr_pages) << PAGE_SHIFT) - 1); memory_notify(MEM_CANCEL_ONLINE, &arg); - mem_hotplug_done(); return ret; } #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ @@ -1170,22 +1167,20 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online) /* create new memmap entry */ firmware_map_add_hotplug(start, start + size, "System RAM"); - /* device_online() will take the lock when calling online_pages() */ - mem_hotplug_done(); - /* online pages if requested */ if (online) walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL, online_memory_block); - - return ret; + goto out; error: /* rollback pgdat allocation and others */ if (new_pgdat && pgdat) rollback_node_hotadd(nid, pgdat); memblock_remove(start, size); + +out: mem_hotplug_done(); return ret; } @@ -1626,16 +1621,10 @@ static int __ref __offline_pages(unsigned long start_pfn, return -EINVAL; if (!IS_ALIGNED(end_pfn, pageblock_nr_pages)) return -EINVAL; - - mem_hotplug_begin(); - /* This makes hotplug much easier...and readable. we assume this for now. .*/ - if (!test_pages_in_a_zone(start_pfn, end_pfn, &valid_start, - &valid_end)) { - mem_hotplug_done(); + if (!test_pages_in_a_zone(start_pfn, end_pfn, &valid_start, &valid_end)) return -EINVAL; - } zone = page_zone(pfn_to_page(valid_start)); node = zone_to_nid(zone); @@ -1644,10 +1633,8 @@ static int __ref __offline_pages(unsigned long start_pfn, /* set above range as isolated */ ret = start_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE, true); - if (ret) { - mem_hotplug_done(); + if (ret) return ret; - } arg.start_pfn = start_pfn; arg.nr_pages = nr_pages; @@ -1718,7 +1705,6 @@ static int __ref __offline_pages(unsigned long start_pfn, writeback_set_ratelimit(); memory_notify(MEM_OFFLINE, &arg); - mem_hotplug_done(); return 0; failed_removal: @@ -1728,10 +1714,10 @@ static int __ref __offline_pages(unsigned long start_pfn, memory_notify(MEM_CANCEL_OFFLINE, &arg); /* pushback to free area */ undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE); - mem_hotplug_done(); return ret; } +/* Must be protected by mem_hotplug_begin() or a device_lock */ int offline_pages(unsigned long start_pfn, unsigned long nr_pages) { return __offline_pages(start_pfn, start_pfn + nr_pages);