On 19.06.19 11:01, Michal Hocko wrote: > On Wed 19-06-19 10:54:08, David Hildenbrand wrote: >> On 19.06.19 08:10, Michal Hocko wrote: >>> On Tue 18-06-19 10:40:06, David Hildenbrand wrote: >>>> On 18.06.19 10:32, Wei Yang wrote: >>>>> On Tue, Jun 18, 2019 at 09:49:48AM +0200, Oscar Salvador wrote: >>>>>> On Tue, Jun 18, 2019 at 08:55:37AM +0800, Wei Yang wrote: >>>>>>> In case of NODE_NOT_IN_PAGE_FLAGS is set, we store section's node id in >>>>>>> section_to_node_table[]. While for hot-add memory, this is missed. >>>>>>> Without this information, page_to_nid() may not give the right node id. >>>>>>> >>>>>>> BTW, current online_pages works because it leverages nid in memory_block. >>>>>>> But the granularity of node id should be mem_section wide. >>>>>> >>>>>> I forgot to ask this before, but why do you mention online_pages here? >>>>>> IMHO, it does not add any value to the changelog, and it does not have much >>>>>> to do with the matter. >>>>>> >>>>> >>>>> Since to me it is a little confused why we don't set the node info but still >>>>> could online memory to the correct node. It turns out we leverage the >>>>> information in memblock. >>>> >>>> I'd also drop the comment here. >>>> >>>>> >>>>>> online_pages() works with memblock granularity and not section granularity. >>>>>> That memblock is just a hot-added range of memory, worth of either 1 section or multiple >>>>>> sections, depending on the arch or on the size of the current memory. >>>>>> And we assume that each hot-added memory all belongs to the same node. >>>>>> >>>>> >>>>> So I am not clear about the granularity of node id. section based or memblock >>>>> based. Or we have two cases: >>>>> >>>>> * for initial memory, section wide >>>>> * for hot-add memory, mem_block wide >>>> >>>> It's all a big mess. Right now, you can offline initial memory with >>>> mixed nodes. Also on my list of many ugly things to clean up. >>>> >>>> (I even remember that we can have mixed nodes within a section, but I >>>> haven't figured out yet how that is supposed to work in some scenarios) >>> >>> Yes, that is indeed the case. See 4aa9fc2a435abe95a1e8d7f8c7b3d6356514b37a. >>> How to fix this? Well, I do not think we can. Section based granularity >>> simply doesn't agree with the reality and so we have to live with that. >>> There is a long way to remove all those section size assumptions from >>> the code though. >>> >> >> Trying to remove NODE_NOT_IN_PAGE_FLAGS could work, but we would have to >> identify how exactly needs that. For memory blocks, we need a different >> approach (I have in my head to make ->nid indicate if we are dealing >> with mixed nodes. If mixed, disallow onlining/offlining). > > Well, I am not sure we really have to care about mutli-nodes memblocks > much. The API is clumsy but does anybody actually care? The vast > majority of hotplug usecases simply do not do that in the first place > right? Yes, AFAIK it could be done, resulting in unpredictable outcome. And if they do need a smaller granularity to describe their > memory topology then we need a different user API rather the fiddle with > implementation details I would argue. > It is not about supporting it, it is about properly blocking it. -- Thanks, David / dhildenb
