Messages in this thread | | | Subject | Re: [PATCH v1] mm/memory_hotplug: Easier calculation to get pages to next section boundary | From | David Hildenbrand <> | Date | Thu, 6 Feb 2020 10:01:21 +0100 |
| |
On 06.02.20 05:39, Baoquan He wrote: > On 02/06/20 at 04:34am, Wei Yang wrote: >> On Thu, Feb 06, 2020 at 10:48:16AM +0800, Baoquan He wrote: >>> On 02/06/20 at 02:26am, Wei Yang wrote: >>>> On Thu, Feb 06, 2020 at 08:37:36AM +0800, Baoquan He wrote: >>>>> On 02/06/20 at 08:13am, Baoquan He wrote: >>>>>> On 02/06/20 at 07:50am, Wei Yang wrote: >>>>>>> On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote: >>>>>>>> On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote: >>>>>>>>> Let's use a calculation that's easier to understand and calculates the >>>>>>>>> same result. Reusing existing macros makes this look nicer. >>>>>>>>> >>>>>>>>> We always want to have the number of pages (> 0) to the next section >>>>>>>>> boundary, starting from the current pfn. >>>>>>>>> >>>>>>>>> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org> >>>>>>>>> Cc: Andrew Morton <akpm@linux-foundation.org> >>>>>>>>> Cc: Michal Hocko <mhocko@kernel.org> >>>>>>>>> Cc: Oscar Salvador <osalvador@suse.de> >>>>>>>>> Cc: Baoquan He <bhe@redhat.com> >>>>>>>>> Cc: Wei Yang <richardw.yang@linux.intel.com> >>>>>>>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>>>>>> >>>>>>>> Reviewed-by: Wei Yang <richardw.yang@linux.intel.com> >>>>>>>> >>>>>>>> BTW, I got one question about hotplug size requirement. >>>>>>>> >>>>>>>> I thought the hotplug range should be section size aligned, while taking a >>>>>>>> look into current code function check_hotplug_memory_range() guard the range. >>>>>> >>>>>> A good question. The current code should be block size aligned. I >>>>>> remember in some places we assume each block comprise all the sections. >>>>>> Can't imagine one or some of them are half section filled. >>>>> >>>>> I could be wrong, half filled block may not cause problem. >>>>> >>>> >>>> David must be angry about our flooding the mail list :-) >>> >>> Believe he won't, :-) If you like, we can talk off line. >>> >>>> >>>> Check the code again, there are two memory range check: >>>> >>>> * check_hotplug_memory_range(), block/section aligned >>>> * check_pfn_span(), subsection aligned >>>> >>>> The second check, check_pfn_span() in __add_pages(), enable the capability to >>>> add a memory range with subsection size. >>>> >>>> This means hotplug still keeps section alignment. >>> >>> memremap_pages() also call add_pages(), it doesn't have the >>> check_hotplug_memory_range() invocation. check_pfn_span() is made for >>> it specifically. >>> >> >> If my understanding is correct, memremap_pages() is used to add some dev >> memory to system. This is the use case which Dan want to enable for >> sub-section. Since memremap_pages() is not called in mem-hotplug path, this >> doesn't affect the hotplug range alignment. > > Yeah, we are on the same page.
We allow sub-section hoy-add only for device memory/hmm. BIOS often align PMEM devices to sub-sections, and not supporting this makes life difficult to support these devices. (You cannot simply cut off something of a disk :) )
System memory can only be hotplugged in memory block granularity, the same granularity onlining/offlining from user space will happen. Boot memory, however, can be sub-section aligned, but can never be offlined (as it contains holes) and therefore never be removed.
>> >>>> >>>> BTW, __add_pages() share the same logic as __remove_pages(). Why not change it >>>> too? Do I miss something or I don't have the latest source code? >>> >>> Good question, and I think it need. Just David is refactoring/cleaning >>> up the remove_pages() code path, this is found out by Segher from patch >>> reviewing. >> >> Ah, we may need a following cleanup :-) > > Agree. See what David will say. Fold it into this patch, or anyone post > a new one. >
Yes, I only cleaned up __remove_pages() for now. I can send a similar cleanup for __add_pages().
Will resend this patch, also taking care of __add_pages(). Thanks!
-- Thanks,
David / dhildenb
| |