Re: [PATCH v2] mm/sparse: set section nid for hot-add memory

[David Hildenbrand <david@redhat.com>]

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).

-- 

Thanks,

David / dhildenb