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@xxxxxxxxxxxxxxxxxxx> >>>>>>>>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >>>>>>>>> Cc: Michal Hocko <mhocko@xxxxxxxxxx> >>>>>>>>> Cc: Oscar Salvador <osalvador@xxxxxxx> >>>>>>>>> Cc: Baoquan He <bhe@xxxxxxxxxx> >>>>>>>>> Cc: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx> >>>>>>>>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx> >>>>>>>> >>>>>>>> Reviewed-by: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx> >>>>>>>> >>>>>>>> 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
