Re: [PATCH] mm/damon/core: remove unnecessary si_meminfo invoke.

[杨欢 <link@vivo.com>]

在 2023/9/18 19:11, SeongJae Park 写道:
> Hi Huan,
>
> On Mon, 18 Sep 2023 17:49:34 +0800 Huan Yang <link@vivo.com> wrote:
>
>> si_meminfo() will read and assign more info not just free/ram pages.
> Nice catch :)
>
>> For just DAMOS_WMARK_FREE_MEM_RATE use, only get free and ram pages
>> is ok to save cpu.
>>
>> Signed-off-by: Huan Yang <link@vivo.com>
>> ---
>>   mm/damon/core.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/damon/core.c b/mm/damon/core.c
>> index bcd2bd9d6c10..1cddee9ae73b 100644
>> --- a/mm/damon/core.c
>> +++ b/mm/damon/core.c
>> @@ -1278,14 +1278,16 @@ static bool kdamond_need_stop(struct damon_ctx *ctx)
>>   	return true;
>>   }
>>   
>> -static unsigned long damos_wmark_metric_value(enum damos_wmark_metric metric)
>> +static unsigned long __damons_get_wmark_free_mem_rate(void)
> Nit.  s/damons/damos/ would look more consistently, in my opinion?
HI, SJ, sorry, what's this mean?
>
>>   {
>> -	struct sysinfo i;
>> +	return global_zone_page_state(NR_FREE_PAGES) * 1000 / totalram_pages();
>> +}
>>   
>> +static unsigned long damos_wmark_metric_value(enum damos_wmark_metric metric)
>> +{
>>   	switch (metric) {
>>   	case DAMOS_WMARK_FREE_MEM_RATE:
>> -		si_meminfo(&i);
>> -		return i.freeram * 1000 / i.totalram;
>> +		return __damons_get_wmark_free_mem_rate();
> Since __damons_get_wmark_free_mem_rate() is just one line function and
> damos_wmark_metric_value() is the only user of the code, I think we could just
> writ the code here?

I do this in mine first patch, but then, I fold this into 
"__damons_get_wmark_free_mem_rate"

due to I think the "__damons_get_wmark_free_mem_rate" may change the 
meaning for furture,

and may si_meminfo will come back soon?(If we need more info to get the 
rate?). And, also, the

static function If just some user use, it will be inline, so, I just 
think fold it will be better.

Do you think so?

Thanks,

Huan

>
>>   	default:
>>   		break;
>>   	}
>> -- 
>> 2.34.1
>
> Thanks,
> SJ