Re: [PATCH v3 6/7] mm: make clear_huge_page cache clear only aroundthe fault address

From: Andrea Arcangeli
Date: Thu Aug 16 2012 - 12:17:48 EST


Hi Kirill,

On Thu, Aug 16, 2012 at 06:15:53PM +0300, Kirill A. Shutemov wrote:
> for (i = 0; i < pages_per_huge_page;
> i++, p = mem_map_next(p, page, i)) {

It may be more optimal to avoid a multiplication/shiftleft before the
add, and to do:

for (i = 0, vaddr = haddr; i < pages_per_huge_page;
i++, p = mem_map_next(p, page, i), vaddr += PAGE_SIZE) {

> cond_resched();
> - clear_user_highpage(p, addr + i * PAGE_SIZE);
> + vaddr = haddr + i*PAGE_SIZE;

Not sure if gcc can optimize it away because of the external calls.

> + if (!ARCH_HAS_USER_NOCACHE || i == target)
> + clear_user_highpage(page + i, vaddr);
> + else
> + clear_user_highpage_nocache(page + i, vaddr);
> }


My only worry overall is if there can be some workload where this may
actually slow down userland if the CPU cache is very large and
userland would access most of the faulted in memory after the first
fault.

So I wouldn't mind to add one more check in addition of
!ARCH_HAS_USER_NOCACHE above to check a runtime sysctl variable. It'll
waste a cacheline yes but I doubt it's measurable compared to the time
it takes to do a >=2M hugepage copy.

Furthermore it would allow people to benchmark its effect without
having to rebuild the kernel themself.

All other patches looks fine to me.

Thanks!
Andrea
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/