From patchwork Tue May 21 12:57:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brendan Jackman X-Patchwork-Id: 13669414 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B8214C25B75 for ; Tue, 21 May 2024 12:57:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4D9B96B008A; Tue, 21 May 2024 08:57:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 488FA6B0092; Tue, 21 May 2024 08:57:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 350FF6B0093; Tue, 21 May 2024 08:57:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 125236B008A for ; Tue, 21 May 2024 08:57:36 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 8325D1211FB for ; Tue, 21 May 2024 12:57:35 +0000 (UTC) X-FDA: 82142404470.18.D14507E Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) by imf03.hostedemail.com (Postfix) with ESMTP id C3B6220002 for ; Tue, 21 May 2024 12:57:31 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cFWDpUmo; spf=pass (imf03.hostedemail.com: domain of 3OppMZggKCNkE57FH5I6BJJBG9.7JHGDIPS-HHFQ57F.JMB@flex--jackmanb.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3OppMZggKCNkE57FH5I6BJJBG9.7JHGDIPS-HHFQ57F.JMB@flex--jackmanb.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716296251; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=tRM9o0p6I8YJ1VI9X9CkGkomZF/jTW7vEZ+k3El+rfw=; b=OaHkZZTNNCy4kTPIKgpSrz2VGOnQ4eXOGRpifkPfa00WPLj7K5vh56sKVLpOBhMqBCbpYV 6Um7eBtppxEUGBdem8cZm7KCkjReOg6Jkl72NKn4NH04OhWKK9zH5n3CXcSPaG9HxAQLaz vhOtnpVrQNbriC6M9gLNase6K/XOEUk= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cFWDpUmo; spf=pass (imf03.hostedemail.com: domain of 3OppMZggKCNkE57FH5I6BJJBG9.7JHGDIPS-HHFQ57F.JMB@flex--jackmanb.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3OppMZggKCNkE57FH5I6BJJBG9.7JHGDIPS-HHFQ57F.JMB@flex--jackmanb.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716296251; a=rsa-sha256; cv=none; b=lYjD2MO159d+6U9fEaDeIFlDY8o1oxsVwXXzPwI/MBFMDs4uHwQvsj8YzjBw/nvW/KKKY+ YU9hOwdNohK8JCNwxcdyBSLA46sOAl8SbI59JMWQ79nh01Gu2/wzH/pzQhsfgFLJBq11OI 0IvNKj3NJw25ia+AddQEil50sK3+2/g= Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-de60321ce6cso23142089276.1 for ; Tue, 21 May 2024 05:57:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1716296251; x=1716901051; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=tRM9o0p6I8YJ1VI9X9CkGkomZF/jTW7vEZ+k3El+rfw=; b=cFWDpUmox/04aijz+08CiLZG9LlHHR6Cmvecte2kxTeH7KCzROFylWmG4aUGgRfNdG vpkL8RbKODc2r1/zUidQwgOvtm7dr8SEl2R+SkVR++EPgln7rxXJ0KTdq5lEJsBG0Hzp /89BvVNUQMaY52tQj19TDQp1grX9QvS1RBRPQW5EE+4hAH5O1llQCJZGOyHQnU0CtyEy RO6wpb1It5R4EyLM3ZOEr2WRtoV1Sd+ValduG7zOqAs/wMtXrFsi7hfiLP/Yc0t0QaQr mu/5d6D2uqhJHKWnrXdqnWVGGpcHOhYJIzgkspFGFyACthKAqG4YhbnvNBYoE3U2XB4G fp+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716296251; x=1716901051; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=tRM9o0p6I8YJ1VI9X9CkGkomZF/jTW7vEZ+k3El+rfw=; b=xID2AXwjHPrYBE8XvN6QxuAXomwMpS1Aei8C+V3nwXmcmUuOInRzC1EfGtBeC7gokb TuRGcehoTq5qkvITaKkNJSKjeWJkWsfwyH8F01XfBkxXjN5aJw8xYOB87nC6gmlhigFH aPB8C6UX2nGupWbxD2aazCQPThudDJOABzzef1eRUTzJ9TxPTCxyXi/90D3MpUpt59bd TmqjWeEV9FgTzb/c/34gjhCyj0Dwp8AnKVAdUKJ9B6tBr8kXVoWJbC0RjR8AYmM9Ettu LzlQo08YU4WcQm0rn10wRqyZB+jIDCtFiNPP2LCyZ112BBCWSwtQ+uHS9IqVLInhLol3 xv6Q== X-Forwarded-Encrypted: i=1; AJvYcCXeLGmTKkYgCEM0LMy/xChoETTpKa3O3ppCCHRnQi+uHNBWMX6yc4bCNTRuqNCdqJXdv+DPmzAkeU8xXamOgxJVqyc= X-Gm-Message-State: AOJu0YwKO/yRsx9eVOYzRCA5nJjYXpOnxgvvj5nhEor5gFlTAYc+UBmW Rs2tmdsAxr011m4TQxUSrQEEHUvcXeODl552/J6CL0cJwJph0dda6EIfNNKVx5CooXGmTiya2wy rQJkmhxYRsQ== X-Google-Smtp-Source: AGHT+IHUQuVOIfwVncwQOFUHVovBLz5PWrCe4Dx+2LOZnSScbdDQHrTlBjLZBbELeDi9NMGgMHKZKXynUK6x6Q== X-Received: from beeg.c.googlers.com ([fda3:e722:ac3:cc00:28:9cb1:c0a8:11db]) (user=jackmanb job=sendgmr) by 2002:a05:6902:120f:b0:dcb:e982:4e40 with SMTP id 3f1490d57ef6-dee4f38b7cbmr8662141276.12.1716296250742; Tue, 21 May 2024 05:57:30 -0700 (PDT) Date: Tue, 21 May 2024 12:57:18 +0000 In-Reply-To: <20240521-mm-hotplug-sync-v1-0-6d53706c1ba8@google.com> Mime-Version: 1.0 References: <20240521-mm-hotplug-sync-v1-0-6d53706c1ba8@google.com> X-Mailer: b4 0.14-dev Message-ID: <20240521-mm-hotplug-sync-v1-1-6d53706c1ba8@google.com> Subject: [PATCH 1/2] mm,memory_hotplug: Remove un-taken lock From: Brendan Jackman To: David Hildenbrand , Oscar Salvador , Andrew Morton , Mike Rapoport Cc: Michal Hocko , Anshuman Khandual , Vlastimil Babka , Pavel Tatashin , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Brendan Jackman X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: C3B6220002 X-Stat-Signature: eww3ge8cuktg4spti56e5eizsdfnb5w5 X-HE-Tag: 1716296251-622867 X-HE-Meta: U2FsdGVkX18Gjhee2xEtyKOMq5WC7dasrSdvAoFaSgDMeoqCkHQLbWIwrzz50KXNSJlKapO369+b2Ys9aB4FKrnNRRFaFLQU/ielDqLbzPP13KBlpqKi4wkF144Y0MFYR4BmXndXrCQMuIAvDX1T7GbI7tjdnz7imJYueqSQpK69i7AZw0ksHIxyhdtwr9c6nPor18qu8M34vqhSn1BdJN+sqrqwckFW59JFgG/iN5f0p6W5aoNXFR3d8LKEdgAu4tJ151hblRlA5berFGDdjOKuDw6yfq/ZZQxUua07AFoudEsImDxt45LhLMBN19vJqQAth4G7a11Ehmtn3soyBMVn64bfdymBqzulBF+Pd6O56VZO9mdB5E1+Sz29V5RG9wADrGVgTbIv0M4TvICx5gKXFM7pwUDFHe9cwTPmDZp7NfuiDZ78q2/737EnP8py446d2XQB2EqVUhMu/Rkw8bo9XaLnkD/GgmkyXVLUHf7JbgHc17Qd3gbCD5yXj/ijnsEyYby3GEHKBG1i6y8bPAbejuD6zdLt0peAXAx2ANG6/wWAFTe/lfqZf3lCGcypog/K5y5SexhKN4Pl9WttMy0qOqAHxkAz+6RoGw0Ug3+26snAQUL/zco+/D44DdxkbqEclJIISLGfcthcOHLgM+UEJCAg2mwsV+Mj75WfdMktW8jZP6EtIDB7t6ZcsJ5zg64oCKs2t589PHi1oizImHGnCOpG98qUyV8ofV9bqfpXGzZCpgvs39MmmWZPtc3YuTaxzmk2GMPKVmDiZZXwIuDP4ghFFlzSPmhiWC234yqrpgZOxgDtJDqBIRYvGu/uHtVJkVtQWC6ynu3yQAJL3xPuTCNZ33982G8+iRRWMQKzfAFYIQ0gQ6jwO8Uftg1tBgj09kFBc72Pi1yNgJoaZWY253BLE89gwd8IwgefmIKyIz0tkev0+KvBIR5Pm/J2RzgyP5733etwUITYKA+ /khSINaj wWZEhcayMrsqTiARYyy7uSeqQRIE45H0OatJg/atcktQpaKzBhvnn3e0i6QU28P2t+6lTkVPnaLy+2+MfyiKHaU9kyeM1pxklposR/EkO2AE7fkeltJHzSPwO4b2mGkbRsjnB3QMFkPvxqa/OicHBgk8oUGJHiiuhkKsNLwtDTD7msvXjinQ68/AhOQrHPGbVaKjshjjgNZ7Lm5IR24Y4zNXRsHyOL0/PLfyrkokKnAWKamAmtw3xenVr60oA7/7Btc1aEs3y6o1A1YShSoXDud7j9ubW72DKKV/ItPOnP25Qp+Ly3T9h2UkLXmgvKPdm8m6rKunhXESd5/09y5mIQs9K8uOnoGTfqag4VV0OwA2fr2MK1PL/Esr9SsAkYThrWk+UrigVc3OYqO8IPINvfW29ePb7tPUP/eh4Assbua3GW8LFDpK83uJflYcyynKX9/GGVRfGkBNdQG5JqhXC/J9ezTzDyJpBTlEZJim1yJOyKE7RpLgbivBL6ayk34qIESzqIWBKTG0k/PvgoWcn0cuVMPh5ouh16N31n9Uzcc92+XillZBxUO8ZxvoF94EVpCaCCxUTOgt3OjgaXu1MMe9nNjUvGXlbAtAqNoUbMBrEBHO16/zgTIL8qIXyXpqCoLBGqQYq7hjOmAtRHrJIKVM9/RH4N+A0FpWoV77/amzTsI9OtLMDciLADzeErFINrKpk0Kd2s/CzBXwuROHrUrdcOs49QMwxW/lTebRJ7/yWuHEmtRw4B8EKlC7mDtX8JBPswpnTgKpz/ZgCtaqT4i5XTP/IEayKxjBuZ7I4LJjWYJL8zKg72WUJc2WzkYFhFfUeaIpUY4t7KnTmpVYdZGwXJKDpomcxxcjJKHnF11UseHXSxTZ09IJrV5dim6xUqNkCYrQFNoNokeJXdfPElXewief20QNDfSh6ehWRjbsnOutsVDzsybiNVA== 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: List-Subscribe: List-Unsubscribe: It seems that [1] was acked, and the a v2 was written[2] which improved upon it, but got bogged down in discussion of other topics, so the improvements were not included. Then [1] got merged as commit 27cacaad16c5 ("mm,memory_hotplug: drop unneeded locking") and we ended up with locks that get taken for read but never for write. So, let's remove the read locking. Compared to Oscar's original v2[2], I have added a READ_ONCE in page_outside_zone_boundaries; this is a substitute for the compiler barrier that was implied by read_seqretry(). I believe this is necessary to insure against UB, although the value being read here is only used for a printk so the stakes seem very low (and this is all debug code anyway). I believe a compiler barrier is also needed in zone_spans_pfn, but I'll address that in a separate patch. That read_seqretry() also impleied a CPU-level memory barrier, which I don't think needs replacing: page_outside_zone_boundaries() is used in the alloc and free paths, but you can't allocate or free pages from the span that is in the middle of being added/removed by hotplug. In other words, page_outside_zone_boundaries() doesn't require a strictly up-to-date view of spanned_pages, but I think it does require a value that was once/will eventually be correct, hence READ_ONCE. [1] https://lore.kernel.org/all/20210531093958.15021-1-osalvador@suse.de/T/#u [2] https://lore.kernel.org/linux-mm/20210602091457.17772-3-osalvador@suse.de/#t Cc: David Hildenbrand Cc: Michal Hocko Cc: Anshuman Khandual Cc: Vlastimil Babka Cc: Pavel Tatashin Co-developed-by: Oscar Salvador Signed-off-by: Oscar Salvador Signed-off-by: Brendan Jackman --- include/linux/memory_hotplug.h | 35 ----------------------------------- include/linux/mmzone.h | 23 +++++------------------ mm/mm_init.c | 1 - mm/page_alloc.c | 10 +++------- 4 files changed, 8 insertions(+), 61 deletions(-) diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h index 7a9ff464608d..f9577e67e5ee 100644 --- a/include/linux/memory_hotplug.h +++ b/include/linux/memory_hotplug.h @@ -141,31 +141,7 @@ bool mhp_supports_memmap_on_memory(void); /* * Zone resizing functions - * - * Note: any attempt to resize a zone should has pgdat_resize_lock() - * zone_span_writelock() both held. This ensure the size of a zone - * can't be changed while pgdat_resize_lock() held. */ -static inline unsigned zone_span_seqbegin(struct zone *zone) -{ - return read_seqbegin(&zone->span_seqlock); -} -static inline int zone_span_seqretry(struct zone *zone, unsigned iv) -{ - return read_seqretry(&zone->span_seqlock, iv); -} -static inline void zone_span_writelock(struct zone *zone) -{ - write_seqlock(&zone->span_seqlock); -} -static inline void zone_span_writeunlock(struct zone *zone) -{ - write_sequnlock(&zone->span_seqlock); -} -static inline void zone_seqlock_init(struct zone *zone) -{ - seqlock_init(&zone->span_seqlock); -} extern void adjust_present_page_count(struct page *page, struct memory_group *group, long nr_pages); @@ -251,17 +227,6 @@ static inline void pgdat_kswapd_lock_init(pg_data_t *pgdat) ___page; \ }) -static inline unsigned zone_span_seqbegin(struct zone *zone) -{ - return 0; -} -static inline int zone_span_seqretry(struct zone *zone, unsigned iv) -{ - return 0; -} -static inline void zone_span_writelock(struct zone *zone) {} -static inline void zone_span_writeunlock(struct zone *zone) {} -static inline void zone_seqlock_init(struct zone *zone) {} static inline int try_online_node(int nid) { diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 8f9c9590a42c..194ef7fed9d6 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -896,18 +895,11 @@ struct zone { * * Locking rules: * - * zone_start_pfn and spanned_pages are protected by span_seqlock. - * It is a seqlock because it has to be read outside of zone->lock, - * and it is done in the main allocator path. But, it is written - * quite infrequently. - * - * The span_seq lock is declared along with zone->lock because it is - * frequently read in proximity to zone->lock. It's good to - * give them a chance of being in the same cacheline. - * - * Write access to present_pages at runtime should be protected by - * mem_hotplug_begin/done(). Any reader who can't tolerant drift of - * present_pages should use get_online_mems() to get a stable value. + * Besides system initialization functions, memory-hotplug is the only + * user that can change zone's {spanned,present} pages at runtime, and + * it does so by holding the mem_hotplug_lock lock. Any readers who + * can't tolerate drift values should use {get,put}_online_mems to get + * a stable value. */ atomic_long_t managed_pages; unsigned long spanned_pages; @@ -930,11 +922,6 @@ struct zone { unsigned long nr_isolate_pageblock; #endif -#ifdef CONFIG_MEMORY_HOTPLUG - /* see spanned/present_pages for more description */ - seqlock_t span_seqlock; -#endif - int initialized; /* Write-intensive fields used from the page allocator */ diff --git a/mm/mm_init.c b/mm/mm_init.c index f72b852bd5b8..c725618aeb58 100644 --- a/mm/mm_init.c +++ b/mm/mm_init.c @@ -1383,7 +1383,6 @@ static void __meminit zone_init_internals(struct zone *zone, enum zone_type idx, zone->name = zone_names[idx]; zone->zone_pgdat = NODE_DATA(nid); spin_lock_init(&zone->lock); - zone_seqlock_init(zone); zone_pcp_init(zone); } diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 2e22ce5675ca..5116a2b9ea6e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -426,16 +426,12 @@ void set_pageblock_migratetype(struct page *page, int migratetype) static int page_outside_zone_boundaries(struct zone *zone, struct page *page) { int ret; - unsigned seq; unsigned long pfn = page_to_pfn(page); unsigned long sp, start_pfn; - do { - seq = zone_span_seqbegin(zone); - start_pfn = zone->zone_start_pfn; - sp = zone->spanned_pages; - ret = !zone_spans_pfn(zone, pfn); - } while (zone_span_seqretry(zone, seq)); + start_pfn = zone->zone_start_pfn; + sp = READ_ONCE(zone->spanned_pages); + ret = !zone_spans_pfn(zone, pfn); if (ret) pr_err("page 0x%lx outside node %d zone %s [ 0x%lx - 0x%lx ]\n",