lkml.org 
[lkml]   [2019]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [v4 PATCH 2/2] mm: vmscan: correct some vmscan counters for
From
Date


On 5/24/19 1:51 PM, Hillf Danton wrote:
> On Fri, 24 May 2019 09:27:02 +0800 Yang Shi wrote:
>> On 5/23/19 11:51 PM, Hillf Danton wrote:
>>> On Thu, 23 May 2019 10:27:38 +0800 Yang Shi wrote:
>>>> @ -1642,14 +1650,14 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>>> unsigned long nr_zone_taken[MAX_NR_ZONES] = { 0 };
>>>> unsigned long nr_skipped[MAX_NR_ZONES] = { 0, };
>>>> unsigned long skipped = 0;
>>>> - unsigned long scan, total_scan, nr_pages;
>>>> + unsigned long scan, total_scan;
>>>> + unsigned long nr_pages;
>>> Change for no earn:)
>> Aha, yes.
>>
>>>> LIST_HEAD(pages_skipped);
>>>> isolate_mode_t mode = (sc->may_unmap ? 0 : ISOLATE_UNMAPPED);
>>>> + total_scan = 0;
>>>> scan = 0;
>>>> - for (total_scan = 0;
>>>> - scan < nr_to_scan && nr_taken < nr_to_scan && !list_empty(src);
>>>> - total_scan++) {
>>>> + while (scan < nr_to_scan && !list_empty(src)) {
>>>> struct page *page;
>>> AFAICS scan currently prevents us from looping for ever, while nr_taken bails
>>> us out once we get what's expected, so I doubt it makes much sense to cut
>>> nr_taken off.
>> It is because "scan < nr_to_scan && nr_taken >= nr_to_scan" is
>> impossible now with the units fixed.
>>
> With the units fixed, nr_taken is no longer checked.

It is because scan would be always >= nr_taken.

>
>>>> page = lru_to_page(src);
>>>> @@ -1657,9 +1665,12 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>>> VM_BUG_ON_PAGE(!PageLRU(page), page);
>>>> + nr_pages = 1 << compound_order(page);
>>>> + total_scan += nr_pages;
>>>> +
>>>> if (page_zonenum(page) > sc->reclaim_idx) {
>>>> list_move(&page->lru, &pages_skipped);
>>>> - nr_skipped[page_zonenum(page)]++;
>>>> + nr_skipped[page_zonenum(page)] += nr_pages;
>>>> continue;
>>>> }
>>>> @@ -1669,10 +1680,9 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>>> * ineligible pages. This causes the VM to not reclaim any
>>>> * pages, triggering a premature OOM.
>>>> */
>>>> - scan++;
>>>> + scan += nr_pages;
>>> The comment looks to defy the change if we fail to add a huge page to
>>> the dst list; otherwise nr_taken knows how to do the right thing. What
>>> I prefer is to let scan to do one thing a time.
>> I don't get your point. Do you mean the comment "Do not count skipped
>> pages because that makes the function return with no isolated pages if
>> the LRU mostly contains ineligible pages."? I'm supposed the comment is
>> used to explain why not count skipped page.
>>
> Well consider the case where there is a huge page in the second place
> reversely on the src list along with other 20 regular pages, and we are
> not able to add the huge page to the dst list. Currently we can go on and
> try to scan other pages, provided nr_to_scan is 32; with the units fixed,
> however, scan goes over nr_to_scan, leaving us no chance to scan any page
> that may be not busy. I wonder that triggers a premature OOM, because I
> think scan means the number of list nodes we try to isolate, and
> nr_taken the number of regular pages successfully isolated.

Yes, good point. I think I just need roll back to what v3 did here to
get scan accounted for each case separately to avoid the possible
over-account.

>>>> switch (__isolate_lru_page(page, mode)) {
>>>> case 0:
>>>> - nr_pages = hpage_nr_pages(page);
>>>> nr_taken += nr_pages;
>>>> nr_zone_taken[page_zonenum(page)] += nr_pages;
>>>> list_move(&page->lru, dst);
>>>> --
>>>> 1.8.3.1
> Best Regards
> Hillf

\
 
 \ /
  Last update: 2019-05-24 08:00    [W:0.029 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site