Re: [PATCH] mm/vmscan: remove pglist_data->inactive_ratio

[Ganesh Mahendran]

Hi, Michal

2016-07-19 15:34 GMT+08:00 Michal Hocko <mhocko@xxxxxxxxxx>:
> On Tue 19-07-16 10:07:29, Ganesh Mahendran wrote:
>> In patch [1], the inactive_ratio is now automatically calculated
>
> It is better to give the direct reference to the patch 59dc76b0d4df
> ("mm: vmscan: reduce size of inactive file list")

Yes, I will change in next version.

>
>> in inactive_list_is_low(). So there is no need to keep inactive_ratio
>> in pglist_data,
>
> OK
>
>> and shown in zoneinfo.
>
> I am not so sure about this. To be honest I have never really used this
> value but maybe there is somebody outher who relies on it. It would be
> safer if the ratio calculation in inactive_list_is_low would be
> extracted and used to display the information rather than dropping that
> on the floor.

Yes, It is not good to remove the inactive_ratio from /proc/zoneinfo.

I will send a V2 patch soon.

Thanks.

>
> The patch should also state that the above patch has broken the zoneinfo
> information.
>
>> [1] mm: vmscan: reduce size of inactive file list
>>
>> Signed-off-by: Ganesh Mahendran <opensource.ganesh@xxxxxxxxx>
>> ---
>>  include/linux/mmzone.h | 6 ------
>>  mm/vmscan.c            | 2 +-
>>  mm/vmstat.c            | 6 ++----
>>  3 files changed, 3 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index a3b7f45..b3ade54 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -700,12 +700,6 @@ typedef struct pglist_data {
>>       /* Fields commonly accessed by the page reclaim scanner */
>>       struct lruvec           lruvec;
>>
>> -     /*
>> -      * The target ratio of ACTIVE_ANON to INACTIVE_ANON pages on
>> -      * this node's LRU.  Maintained by the pageout code.
>> -      */
>> -     unsigned int inactive_ratio;
>> -
>>       unsigned long           flags;
>>
>>       ZONE_PADDING(_pad2_)
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 429bf3a..3c1de58 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -1915,7 +1915,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
>>   * page has a chance to be referenced again before it is reclaimed.
>>   *
>>   * The inactive_ratio is the target ratio of ACTIVE to INACTIVE pages
>> - * on this LRU, maintained by the pageout code. A zone->inactive_ratio
>> + * on this LRU, maintained by the pageout code. A inactive_ratio
>>   * of 3 means 3:1 or 25% of the pages are kept on the inactive list.
>>   *
>>   * total     target    max
>> diff --git a/mm/vmstat.c b/mm/vmstat.c
>> index 91ecca9..74a0eca 100644
>> --- a/mm/vmstat.c
>> +++ b/mm/vmstat.c
>> @@ -1491,11 +1491,9 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
>>       }
>>       seq_printf(m,
>>                  "\n  node_unreclaimable:  %u"
>> -                "\n  start_pfn:           %lu"
>> -                "\n  node_inactive_ratio: %u",
>> +                "\n  start_pfn:           %lu",
>>                  !pgdat_reclaimable(zone->zone_pgdat),
>> -                zone->zone_start_pfn,
>> -                zone->zone_pgdat->inactive_ratio);
>> +                zone->zone_start_pfn);
>>       seq_putc(m, '\n');
>>  }
>>
>> --
>> 1.9.1
>>
>
> --
> Michal Hocko
> SUSE Labs