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

[Minchan Kim]

Hi, Hugh.

On Thu, Dec 31, 2009 at 2:03 AM, Hugh Dickins
<hugh.dickins@xxxxxxxxxxxxx> wrote:
> 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?

I mentioned zero page was not moving around the LRU in changelog.
The concern was just unecessary call of mark_page_accessed in zero page.

>
> 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.

Indeed. If it is only one  to check, I might insist on this with
performance check.
But if we have many palce to check, this case-by-case approach is a not good.

>
> 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.

Okay. Because you and Rik who have many experience in real workload
dislike it and
I have no number, I will drop this.

Thanks for all reviewers.

>
> Hugh
>



-- 
Kind regards,
Minchan Kim
--
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/