Re: [PATCH -mmotm-2009-12-10-17-19] Prevent churning of zero pagein LRU list.

[Hugh Dickins]

On Mon, 28 Dec 2009, Minchan Kim wrote:
> On Sun, 27 Dec 2009 22:22:20 -0500
> Rik van Riel <riel@xxxxxxxxxx> wrote:
> > On 12/27/2009 09:53 PM, Minchan Kim wrote:
> > >
> > > VM doesn't add zero page to LRU list.
> > > It means zero page's churning in LRU list is pointless.
> > >
> > > As a matter of fact, zero page can't be promoted by mark_page_accessed
> > > since it doesn't have PG_lru.
> > >
> > > This patch prevent unecessary mark_page_accessed call of zero page
> > > alghouth caller want FOLL_TOUCH.
> > >
> > > Signed-off-by: Minchan Kim<minchan.kim@xxxxxxxxx>
> > 
> > The code looks correct, but I wonder how frequently we run into
> > the zero page in this code, vs. how much the added cost is of
> > having this extra code in follow_page.
> > 
> > What kind of problem were you running into that motivated you
> > to write this patch?
> 
> I didn't have experienced any problem in this case. 
> In fact, I found that while trying to make patch smap_pte_change. 
> 
> Long time ago when we have a zero page, we regards it to file_rss. 
> So while we see the smaps, vm_normal_page returns zero page and we can
> calculate it properly with PSS. 
> 
> But now we don't acccout zero page to file_rss. 
> I am not sure we have to account it with file_rss. 
> So I think now smaps_pte_range's resident count routine also is changed. 
> 
> Anyway, I think my patch doesn't have much cost since many customers of 
> follow_page are already not a fast path.
> 
> I tend to agree with your opinion "How frequently we runt into the zero page?"
> But my thought GUP is export function which can be used for anything by anyone.
> 
> Thanks for the review, Rik. 

I'm guessing that you've now dropped the idea of this patch,
since it wasn't included along with your 1/3, 2/3, 3/3.

You thought the ZERO_PAGE was moving around the LRUs, but now
realize that it isn't, so accept there's no need for this patch?

There's lots of places where we could shave a little time off dealing
with the ZERO_PAGE by adding tests for it; but at the expense of
adding code to normal paths of the system, slowing them down.

If there's a proven reason for doing so somewhere, yes, we should
add such tests to avoid significant cacheline bouncing; but without
good reason, we just let ZERO_PAGEs fall through the code as they do.

I believe that get_user_pages() on ZERO_PAGEs is exceptional, beyond
the cases of coredumping and mlock and make_pages_present; but if
you've evidence for adding a test somewhere, please provide it.

Hugh
--
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/