[-next,1/7] mm_types: add _last_cpupid into folio

  At present, only arc/sparc/m68k define WANT_PAGE_VIRTUAL, both of
them don't support numa balancing, and the page struct is aligned
to _struct_page_alignment, it is safe to move _last_cpupid before
'virtual' in page, meanwhile, add it into folio, which make us to
use folio->_last_cpupid directly.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 include/linux/mm_types.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
  

Kefeng Wang <wangkefeng.wang@huawei.com> writes:

> At present, only arc/sparc/m68k define WANT_PAGE_VIRTUAL, both of
> them don't support numa balancing, and the page struct is aligned
> to _struct_page_alignment, it is safe to move _last_cpupid before
> 'virtual' in page, meanwhile, add it into folio, which make us to
> use folio->_last_cpupid directly.

Add BUILD_BUG_ON() to check this automatically?

--
Best Regards,
Huang, Ying
  

On 2023/10/10 16:17, Huang, Ying wrote:
> Kefeng Wang <wangkefeng.wang@huawei.com> writes:
> 
>> At present, only arc/sparc/m68k define WANT_PAGE_VIRTUAL, both of
>> them don't support numa balancing, and the page struct is aligned
>> to _struct_page_alignment, it is safe to move _last_cpupid before
>> 'virtual' in page, meanwhile, add it into folio, which make us to
>> use folio->_last_cpupid directly.
> 
> Add BUILD_BUG_ON() to check this automatically?

The WANT_PAGE_VIRTUAL and LAST_CPUPID_NOT_IN_PAGE_FLAGS are not
conflict, the check is to make sure that the re-order the virtual
and _last_cpupid is minimal impact, and there is a build warning in
mm/memory.c when the LAST_CPUPID_NOT_IN_PAGE_FLAGS is enabled, so I
don't think we need a new BUILD_BUG_ON here.

Thanks.

> 
> --
> Best Regards,
> Huang, Ying
  

On Tue, Oct 10, 2023 at 02:45:38PM +0800, Kefeng Wang wrote:
> At present, only arc/sparc/m68k define WANT_PAGE_VIRTUAL, both of
> them don't support numa balancing, and the page struct is aligned
> to _struct_page_alignment, it is safe to move _last_cpupid before
> 'virtual' in page, meanwhile, add it into folio, which make us to
> use folio->_last_cpupid directly.

What do you mean by "safe"?  I think you mean "Does not increase the
size of struct page", but if that is what you mean, why not just say so?
If there's something else you mean, please explain.

In any event, I'd like to see some reasoning that _last_cpupid is actually
information which is logically maintained on a per-allocation basis,
not a per-page basis (I think this is true, but I honestly don't know)

And looking at all this, I think it makes sense to move _last_cpupid
before the kmsan garbage, then add both 'virtual' and '_last_cpupid'
to folio.
  

On 2023/10/10 20:33, Matthew Wilcox wrote:
> On Tue, Oct 10, 2023 at 02:45:38PM +0800, Kefeng Wang wrote:
>> At present, only arc/sparc/m68k define WANT_PAGE_VIRTUAL, both of
>> them don't support numa balancing, and the page struct is aligned
>> to _struct_page_alignment, it is safe to move _last_cpupid before
>> 'virtual' in page, meanwhile, add it into folio, which make us to
>> use folio->_last_cpupid directly.
> 
> What do you mean by "safe"?  I think you mean "Does not increase the
> size of struct page", but if that is what you mean, why not just say so?
> If there's something else you mean, please explain.

Don't increase size of struct page and don't impact the real order of
struct page as the above three archs without numa balancing support.

> 
> In any event, I'd like to see some reasoning that _last_cpupid is actually
> information which is logically maintained on a per-allocation basis,
> not a per-page basis (I think this is true, but I honestly don't know)

The _last_cpupid is updated in should_numa_migrate_memory() from numa
fault(do_numa_page, and do_huge_pmd_numa_page), it is per-page(normal
page and PMD-mapped page). Maybe I misunderstand your mean, please
correct me.

> 
> And looking at all this, I think it makes sense to move _last_cpupid
> before the kmsan garbage, then add both 'virtual' and '_last_cpupid'
> to folio.

sure, I will add both of them into folio and don't re-order 'virtual' 
and '_last_cpupid'.
> 
>
  

Kefeng Wang <wangkefeng.wang@huawei.com> writes:

> On 2023/10/10 20:33, Matthew Wilcox wrote:
>> On Tue, Oct 10, 2023 at 02:45:38PM +0800, Kefeng Wang wrote:
>>> At present, only arc/sparc/m68k define WANT_PAGE_VIRTUAL, both of
>>> them don't support numa balancing, and the page struct is aligned
>>> to _struct_page_alignment, it is safe to move _last_cpupid before
>>> 'virtual' in page, meanwhile, add it into folio, which make us to
>>> use folio->_last_cpupid directly.
>> What do you mean by "safe"?  I think you mean "Does not increase the
>> size of struct page", but if that is what you mean, why not just say so?
>> If there's something else you mean, please explain.
>
> Don't increase size of struct page and don't impact the real order of
> struct page as the above three archs without numa balancing support.
>
>> In any event, I'd like to see some reasoning that _last_cpupid is
>> actually
>> information which is logically maintained on a per-allocation basis,
>> not a per-page basis (I think this is true, but I honestly don't know)
>
> The _last_cpupid is updated in should_numa_migrate_memory() from numa
> fault(do_numa_page, and do_huge_pmd_numa_page), it is per-page(normal
> page and PMD-mapped page). Maybe I misunderstand your mean, please
> correct me.

Because PTE mapped THP will not be migrated according to comments and
folio_test_large() test in do_numa_page().  Only _last_cpuid of the head
page will be used (that is, on per-allocation basis).  Although in
change_pte_range() in mprotect.c, _last_cpuid of tail pages may be
changed, they are not used actually.  All in all, _last_cpuid is on
per-allocation basis for now.

In the future, it's hard to say.  PTE-mapped THPs or large folios give
us an opportunity to check whether the different parts of a folio are
accessed by multiple sockets, so that we should split the folio.  But
this is just some possibility in the future.

--
Best Regards,
Huang, Ying
  

On 2023/10/11 13:55, Huang, Ying wrote:
> Kefeng Wang <wangkefeng.wang@huawei.com> writes:
> 
>> On 2023/10/10 20:33, Matthew Wilcox wrote:
>>> On Tue, Oct 10, 2023 at 02:45:38PM +0800, Kefeng Wang wrote:
>>>> At present, only arc/sparc/m68k define WANT_PAGE_VIRTUAL, both of
>>>> them don't support numa balancing, and the page struct is aligned
>>>> to _struct_page_alignment, it is safe to move _last_cpupid before
>>>> 'virtual' in page, meanwhile, add it into folio, which make us to
>>>> use folio->_last_cpupid directly.
>>> What do you mean by "safe"?  I think you mean "Does not increase the
>>> size of struct page", but if that is what you mean, why not just say so?
>>> If there's something else you mean, please explain.
>>
>> Don't increase size of struct page and don't impact the real order of
>> struct page as the above three archs without numa balancing support.
>>
>>> In any event, I'd like to see some reasoning that _last_cpupid is
>>> actually
>>> information which is logically maintained on a per-allocation basis,
>>> not a per-page basis (I think this is true, but I honestly don't know)
>>
>> The _last_cpupid is updated in should_numa_migrate_memory() from numa
>> fault(do_numa_page, and do_huge_pmd_numa_page), it is per-page(normal
>> page and PMD-mapped page). Maybe I misunderstand your mean, please
>> correct me.
> 
> Because PTE mapped THP will not be migrated according to comments and
> folio_test_large() test in do_numa_page().  Only _last_cpuid of the head
> page will be used (that is, on per-allocation basis).  Although in
> change_pte_range() in mprotect.c, _last_cpuid of tail pages may be
> changed, they are not used actually.  All in all, _last_cpuid is on
> per-allocation basis for now.

Thanks for clarification, yes, it's what I mean, too
> 
> In the future, it's hard to say.  PTE-mapped THPs or large folios give
> us an opportunity to check whether the different parts of a folio are
> accessed by multiple sockets, so that we should split the folio.  But
> this is just some possibility in the future.

It depends on memory access behavior of application,if multiple sockets
access a large folio/PTE-mappped THP frequently, split maybe better,
or it is enough to just migrate the entire folio.


> 
> --
> Best Regards,
> Huang, Ying
>