On Thu, Aug 16, 2012 at 06:16:47PM +0200, Andrea Arcangeli wrote:
> 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) {
>
Makes sense. I'll update it.
> > 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.
Hm.. I think with static_key we can avoid cache overhead here. I'll try.
> Furthermore it would allow people to benchmark its effect without
> having to rebuild the kernel themself.
>
> All other patches looks fine to me.
Thanks, for review. Could you take a look at huge zero page patchset? ;)
--
Kirill A. Shutemov
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
