From: David Hildenbrand <david@redhat.com> To: Oscar Salvador <osalvador@techadventures.net>, Michal Hocko <mhocko@kernel.org> Cc: pavel.tatashin@microsoft.com, linux-nvdimm@lists.01.org, dave.hansen@intel.com, linux-kernel@vger.kernel.org, mingo@kernel.org, linux-mm@kvack.org, jglisse@redhat.com, rppt@linux.vnet.ibm.com, kirill.shutemov@linux.intel.com, Alexander Duyck <alexander.h.duyck@linux.intel.com>, akpm@linux-foundation.org Subject: Re: [PATCH v5 4/4] mm: Defer ZONE_DEVICE page initialization to the point where we init pgmap Date: Thu, 27 Sep 2018 17:41:19 +0200 [thread overview] Message-ID: <27f1265b-a8b4-b203-077b-468d1e823cc8@redhat.com> (raw) In-Reply-To: <20180927145035.GA21373@techadventures.net> On 27/09/2018 16:50, Oscar Salvador wrote: > On Thu, Sep 27, 2018 at 03:13:29PM +0200, Michal Hocko wrote: >> On Thu 27-09-18 14:25:37, Oscar Salvador wrote: >>> On Thu, Sep 27, 2018 at 01:09:26PM +0200, Michal Hocko wrote: >>>>> So there were a few things I wasn't sure we could pull outside of the >>>>> hotplug lock. One specific example is the bits related to resizing the pgdat >>>>> and zone. I wanted to avoid pulling those bits outside of the hotplug lock. >>>> >>>> Why would that be a problem. There are dedicated locks for resizing. >>> >>> True is that move_pfn_range_to_zone() manages the locks for pgdat/zone resizing, >>> but it also takes care of calling init_currently_empty_zone() in case the zone is empty. >>> Could not that be a problem if we take move_pfn_range_to_zone() out of the lock? >> >> I would have to double check but is the hotplug lock really serializing >> access to the state initialized by init_currently_empty_zone? E.g. >> zone_start_pfn is a nice example of a state that is used outside of the >> lock. zone's free lists are similar. So do we really need the hoptlug >> lock? And more broadly, what does the hotplug lock is supposed to >> serialize in general. A proper documentation would surely help to answer >> these questions. There is way too much of "do not touch this code and >> just make my particular hack" mindset which made the whole memory >> hotplug a giant pile of mess. We really should start with some proper >> engineering here finally. > > CC David > > David has been looking into this lately, he even has updated memory-hotplug.txt > with some more documentation about the locking aspect [1]. > And with this change [2], the hotplug lock has been moved > to the online/offline_pages. > > From what I see (I might be wrong), the hotplug lock is there > to serialize the online/offline operations. mem_hotplug_lock is especially relevant for users of get_online_mems/put_online_mems. Whatever affects them, you can't move out of the lock. Everything else is theoretically serialized via device_hotplug_lock now. > > In online_pages, we do (among other things): > > a) initialize the zone and its pages, and link them to the zone > b) re-adjust zone/pgdat nr of pages (present, spanned, managed) > b) check if the node changes in regard of N_MEMORY, N_HIGH_MEMORY or N_NORMAL_MEMORY. > c) fire notifiers > d) rebuild the zonelists in case we got a new zone > e) online memory sections and free the pages to the buddy allocator > f) wake up kswapd/kcompactd in case we got a new node > > while in offline_pages we do the opposite. > > Hotplug lock here serializes the operations as a whole, online and offline memory, > so they do not step on each other's feet. > > Having said that, we might be able to move some of those operations out of the hotplug lock. > The device_hotplug_lock coming from every memblock (which is taken in device_online/device_offline) should protect > us against some operations being made on the same memblock (e.g: touching the same pages). Yes, very right. -- Thanks, David / dhildenb _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: David Hildenbrand <david@redhat.com> To: Oscar Salvador <osalvador@techadventures.net>, Michal Hocko <mhocko@kernel.org> Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>, linux-mm@kvack.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org, pavel.tatashin@microsoft.com, dave.jiang@intel.com, dave.hansen@intel.com, jglisse@redhat.com, rppt@linux.vnet.ibm.com, dan.j.williams@intel.com, logang@deltatee.com, mingo@kernel.org, kirill.shutemov@linux.intel.com Subject: Re: [PATCH v5 4/4] mm: Defer ZONE_DEVICE page initialization to the point where we init pgmap Date: Thu, 27 Sep 2018 17:41:19 +0200 [thread overview] Message-ID: <27f1265b-a8b4-b203-077b-468d1e823cc8@redhat.com> (raw) In-Reply-To: <20180927145035.GA21373@techadventures.net> On 27/09/2018 16:50, Oscar Salvador wrote: > On Thu, Sep 27, 2018 at 03:13:29PM +0200, Michal Hocko wrote: >> On Thu 27-09-18 14:25:37, Oscar Salvador wrote: >>> On Thu, Sep 27, 2018 at 01:09:26PM +0200, Michal Hocko wrote: >>>>> So there were a few things I wasn't sure we could pull outside of the >>>>> hotplug lock. One specific example is the bits related to resizing the pgdat >>>>> and zone. I wanted to avoid pulling those bits outside of the hotplug lock. >>>> >>>> Why would that be a problem. There are dedicated locks for resizing. >>> >>> True is that move_pfn_range_to_zone() manages the locks for pgdat/zone resizing, >>> but it also takes care of calling init_currently_empty_zone() in case the zone is empty. >>> Could not that be a problem if we take move_pfn_range_to_zone() out of the lock? >> >> I would have to double check but is the hotplug lock really serializing >> access to the state initialized by init_currently_empty_zone? E.g. >> zone_start_pfn is a nice example of a state that is used outside of the >> lock. zone's free lists are similar. So do we really need the hoptlug >> lock? And more broadly, what does the hotplug lock is supposed to >> serialize in general. A proper documentation would surely help to answer >> these questions. There is way too much of "do not touch this code and >> just make my particular hack" mindset which made the whole memory >> hotplug a giant pile of mess. We really should start with some proper >> engineering here finally. > > CC David > > David has been looking into this lately, he even has updated memory-hotplug.txt > with some more documentation about the locking aspect [1]. > And with this change [2], the hotplug lock has been moved > to the online/offline_pages. > > From what I see (I might be wrong), the hotplug lock is there > to serialize the online/offline operations. mem_hotplug_lock is especially relevant for users of get_online_mems/put_online_mems. Whatever affects them, you can't move out of the lock. Everything else is theoretically serialized via device_hotplug_lock now. > > In online_pages, we do (among other things): > > a) initialize the zone and its pages, and link them to the zone > b) re-adjust zone/pgdat nr of pages (present, spanned, managed) > b) check if the node changes in regard of N_MEMORY, N_HIGH_MEMORY or N_NORMAL_MEMORY. > c) fire notifiers > d) rebuild the zonelists in case we got a new zone > e) online memory sections and free the pages to the buddy allocator > f) wake up kswapd/kcompactd in case we got a new node > > while in offline_pages we do the opposite. > > Hotplug lock here serializes the operations as a whole, online and offline memory, > so they do not step on each other's feet. > > Having said that, we might be able to move some of those operations out of the hotplug lock. > The device_hotplug_lock coming from every memblock (which is taken in device_online/device_offline) should protect > us against some operations being made on the same memblock (e.g: touching the same pages). Yes, very right. -- Thanks, David / dhildenb
next prev parent reply other threads:[~2018-09-27 15:41 UTC|newest] Thread overview: 144+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-25 20:18 [PATCH v5 0/4] Address issues slowing persistent memory initialization Alexander Duyck 2018-09-25 20:18 ` Alexander Duyck 2018-09-25 20:19 ` [PATCH v5 1/4] mm: Remove now defunct NO_BOOTMEM from depends list for deferred init Alexander Duyck 2018-09-25 20:19 ` Alexander Duyck 2018-09-25 21:05 ` Mike Rapoport 2018-09-25 21:05 ` Mike Rapoport 2018-09-25 20:20 ` [PATCH v5 2/4] mm: Provide kernel parameter to allow disabling page init poisoning Alexander Duyck 2018-09-25 20:20 ` Alexander Duyck 2018-09-25 20:26 ` Dave Hansen 2018-09-25 20:26 ` Dave Hansen 2018-09-25 20:38 ` Alexander Duyck 2018-09-25 20:38 ` Alexander Duyck 2018-09-25 22:14 ` Dave Hansen 2018-09-25 22:14 ` Dave Hansen 2018-09-25 22:14 ` Dave Hansen 2018-09-25 22:27 ` Alexander Duyck 2018-09-25 22:27 ` Alexander Duyck 2018-09-25 22:27 ` Alexander Duyck 2018-09-26 7:38 ` Michal Hocko 2018-09-26 7:38 ` Michal Hocko 2018-09-26 15:24 ` Alexander Duyck 2018-09-26 15:39 ` Michal Hocko 2018-09-26 15:39 ` Michal Hocko 2018-09-26 15:41 ` Dave Hansen 2018-09-26 15:41 ` Dave Hansen 2018-09-26 16:18 ` Alexander Duyck 2018-09-26 15:36 ` Dave Hansen 2018-09-26 22:36 ` Andrew Morton 2018-09-26 22:36 ` Andrew Morton 2018-09-25 20:20 ` [PATCH v5 3/4] mm: Create non-atomic version of SetPageReserved for init use Alexander Duyck 2018-09-25 20:20 ` Alexander Duyck 2018-09-25 20:21 ` [PATCH v5 4/4] mm: Defer ZONE_DEVICE page initialization to the point where we init pgmap Alexander Duyck 2018-09-25 20:21 ` Alexander Duyck 2018-09-26 7:55 ` Michal Hocko 2018-09-26 18:25 ` Alexander Duyck 2018-09-26 18:25 ` Alexander Duyck 2018-09-26 18:52 ` Dan Williams 2018-09-26 18:52 ` Dan Williams 2018-09-27 11:20 ` Michal Hocko 2018-09-27 11:20 ` Michal Hocko 2018-09-27 11:09 ` Michal Hocko 2018-09-27 11:09 ` Michal Hocko 2018-09-27 12:25 ` Oscar Salvador 2018-09-27 13:13 ` Michal Hocko 2018-09-27 14:50 ` Oscar Salvador 2018-09-27 14:50 ` Oscar Salvador 2018-09-27 14:50 ` Oscar Salvador 2018-09-27 15:41 ` David Hildenbrand [this message] 2018-09-27 15:41 ` David Hildenbrand 2018-09-28 8:12 ` Oscar Salvador 2018-09-28 8:12 ` Oscar Salvador 2018-09-28 8:44 ` Oscar Salvador 2018-09-28 8:44 ` Oscar Salvador 2018-09-28 15:50 ` Dan Williams 2018-09-28 15:50 ` Dan Williams 2018-09-27 12:32 ` Oscar Salvador 2018-10-08 21:01 ` Dan Williams 2018-10-08 21:01 ` Dan Williams 2018-10-08 21:38 ` Alexander Duyck 2018-10-08 21:38 ` Alexander Duyck 2018-10-08 22:00 ` Dan Williams 2018-10-08 22:00 ` Dan Williams 2018-10-08 22:00 ` Dan Williams 2018-10-08 22:07 ` Alexander Duyck 2018-10-08 22:07 ` Alexander Duyck 2018-10-08 22:36 ` Alexander Duyck 2018-10-08 22:36 ` Alexander Duyck 2018-10-08 22:59 ` Dan Williams 2018-10-08 23:34 ` [mm PATCH] memremap: Fix reference count for pgmap in devm_memremap_pages Alexander Duyck 2018-10-08 23:34 ` Alexander Duyck 2018-10-09 0:20 ` Dan Williams 2018-10-09 0:20 ` Dan Williams 2018-10-09 17:00 ` [PATCH v5 4/4] mm: Defer ZONE_DEVICE page initialization to the point where we init pgmap Yi Zhang 2018-10-09 17:00 ` Yi Zhang 2018-10-09 18:04 ` Dan Williams 2018-10-09 18:04 ` Dan Williams 2018-10-09 20:26 ` Alexander Duyck 2018-10-09 20:26 ` Alexander Duyck 2018-10-09 21:19 ` Dan Williams 2018-10-09 21:19 ` Dan Williams 2018-10-10 12:52 ` Yi Zhang 2018-10-10 12:52 ` Yi Zhang 2018-10-10 15:27 ` Alexander Duyck 2018-10-10 15:27 ` Alexander Duyck 2018-10-11 8:17 ` Yi Zhang 2018-10-11 8:17 ` Yi Zhang 2018-10-10 9:58 ` Michal Hocko 2018-10-10 16:39 ` Alexander Duyck 2018-10-10 16:39 ` Alexander Duyck 2018-10-10 17:24 ` Michal Hocko 2018-10-10 17:24 ` Michal Hocko 2018-10-10 17:39 ` Alexander Duyck 2018-10-10 17:39 ` Alexander Duyck 2018-10-10 17:53 ` Michal Hocko 2018-10-10 17:53 ` Michal Hocko 2018-10-10 18:13 ` Alexander Duyck 2018-10-10 18:13 ` Alexander Duyck 2018-10-10 18:52 ` Michal Hocko 2018-10-10 18:52 ` Michal Hocko 2018-10-11 8:55 ` Michal Hocko 2018-10-11 8:55 ` Michal Hocko 2018-10-11 17:38 ` Alexander Duyck 2018-10-11 18:22 ` Dan Williams 2018-10-11 18:22 ` Dan Williams 2018-10-17 7:52 ` Michal Hocko 2018-10-17 7:52 ` Michal Hocko 2018-10-17 15:02 ` Alexander Duyck 2018-10-17 15:02 ` Alexander Duyck 2018-10-29 14:12 ` Michal Hocko 2018-10-29 14:12 ` Michal Hocko 2018-10-29 15:59 ` Alexander Duyck 2018-10-29 15:59 ` Alexander Duyck 2018-10-29 15:59 ` Alexander Duyck 2018-10-29 16:35 ` Michal Hocko 2018-10-29 16:35 ` Michal Hocko 2018-10-29 17:01 ` Alexander Duyck 2018-10-29 17:24 ` Michal Hocko 2018-10-29 17:24 ` Michal Hocko 2018-10-29 17:34 ` Dan Williams 2018-10-29 17:34 ` Dan Williams 2018-10-29 17:45 ` Michal Hocko 2018-10-29 17:45 ` Michal Hocko 2018-10-29 17:42 ` Alexander Duyck 2018-10-29 17:42 ` Alexander Duyck 2018-10-29 18:18 ` Michal Hocko 2018-10-29 18:18 ` Michal Hocko 2018-10-29 19:59 ` Alexander Duyck 2018-10-29 19:59 ` Alexander Duyck 2018-10-30 6:29 ` Michal Hocko 2018-10-30 6:29 ` Michal Hocko 2018-10-30 6:55 ` Dan Williams 2018-10-30 8:17 ` Michal Hocko 2018-10-30 8:17 ` Michal Hocko 2018-10-30 15:57 ` Dan Williams 2018-10-30 8:05 ` Oscar Salvador 2018-10-29 15:49 ` Dan Williams 2018-10-29 15:49 ` Dan Williams 2018-10-29 15:56 ` Michal Hocko 2018-10-10 18:18 ` Dan Williams 2018-10-10 18:18 ` Dan Williams 2018-10-11 8:39 ` Yi Zhang 2018-10-11 8:39 ` Yi Zhang 2018-10-11 15:38 ` Alexander Duyck 2018-10-11 15:38 ` Alexander Duyck
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=27f1265b-a8b4-b203-077b-468d1e823cc8@redhat.com \ --to=david@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=alexander.h.duyck@linux.intel.com \ --cc=dave.hansen@intel.com \ --cc=jglisse@redhat.com \ --cc=kirill.shutemov@linux.intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-nvdimm@lists.01.org \ --cc=mhocko@kernel.org \ --cc=mingo@kernel.org \ --cc=osalvador@techadventures.net \ --cc=pavel.tatashin@microsoft.com \ --cc=rppt@linux.vnet.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.