Re: [PATCH -mm] mm, huge page: Copy to access sub-page last when copy huge page
Mike Kravetz writes: > On 05/17/2018 11:24 PM, Michal Hocko wrote: >> On Fri 18-05-18 11:03:16, Huang, Ying wrote: >> [...] >>> The patch is a generic optimization which should benefit quite some >>> workloads, not for a specific use case. To demonstrate the performance >>> benefit of the patch, we tested it with vm-scalability run on >>> transparent huge page. >> >> It is also adds quite some non-intuitive code. So is this worth? Does >> any _real_ workload benefits from the change? > > One way to 'add less code' would be to create a helper routine that > indicates the order in which sub-pages are to be copied. IIUC, you > added the same algorithm for sub-page ordering to copy_huge_page() > that was previously added to clear_huge_page(). Correct? Yes. > If so, then perhaps a common helper could be used by both the clear > and copy huge page routines. It would also make maintenance easier. That's a good idea. But this may need to turn copy_user_highpage()/clear_user_highpage() calling in copy_user_huge_page()/clear_huge_page() from direct call to indirect call. I don't know whether this will incur some overhead. Will try to measure this. Best Regards, Huang, Ying
Re: [PATCH -mm] mm, huge page: Copy to access sub-page last when copy huge page
Hi, Michal, Michal Hocko writes: > On Fri 18-05-18 11:03:16, Huang, Ying wrote: > [...] >> The patch is a generic optimization which should benefit quite some >> workloads, not for a specific use case. To demonstrate the performance >> benefit of the patch, we tested it with vm-scalability run on >> transparent huge page. > > It is also adds quite some non-intuitive code. So is this worth? Does > any _real_ workload benefits from the change? I don't have any _real_ workload which benefits from this. But I think this is the right way to copy the huge page. It should benefit many workloads with heavy cache contention, as illustrated in the micro-benchmark. But the performance benefit may be small or non-measurable for the _real_ workload. The code does become not as intuitive as before. But fortunately, all non-intuitive code are in copy_user_huge_page(), which is a leaf function with well defined interface and semantics. And with the help of the code comments, at least the intention of the code is clear. Best Regards, Huang, Ying >> include/linux/mm.h | 3 ++- >> mm/huge_memory.c | 3 ++- >> mm/memory.c| 43 +++ >> 3 files changed, 43 insertions(+), 6 deletions(-)
Re: [PATCH -mm] mm, huge page: Copy to access sub-page last when copy huge page
On 05/17/2018 11:24 PM, Michal Hocko wrote: > On Fri 18-05-18 11:03:16, Huang, Ying wrote: > [...] >> The patch is a generic optimization which should benefit quite some >> workloads, not for a specific use case. To demonstrate the performance >> benefit of the patch, we tested it with vm-scalability run on >> transparent huge page. > > It is also adds quite some non-intuitive code. So is this worth? Does > any _real_ workload benefits from the change? One way to 'add less code' would be to create a helper routine that indicates the order in which sub-pages are to be copied. IIUC, you added the same algorithm for sub-page ordering to copy_huge_page() that was previously added to clear_huge_page(). Correct? If so, then perhaps a common helper could be used by both the clear and copy huge page routines. It would also make maintenance easier. -- Mike Kravetz
Re: [PATCH -mm] mm, huge page: Copy to access sub-page last when copy huge page
On Fri 18-05-18 11:03:16, Huang, Ying wrote: [...] > The patch is a generic optimization which should benefit quite some > workloads, not for a specific use case. To demonstrate the performance > benefit of the patch, we tested it with vm-scalability run on > transparent huge page. It is also adds quite some non-intuitive code. So is this worth? Does any _real_ workload benefits from the change? > include/linux/mm.h | 3 ++- > mm/huge_memory.c | 3 ++- > mm/memory.c| 43 +++ > 3 files changed, 43 insertions(+), 6 deletions(-) -- Michal Hocko SUSE Labs
