The totalreserve_pages could be higher than the free because of
watermark high or watermark boost. Handle this situation and fix it to 0
free size.
Signed-off-by: Jaewon Kim <[email protected]>
---
mm/page_alloc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 218b28ee49ed..e510ae83d5f3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5948,6 +5948,8 @@ long si_mem_available(void)
* without causing swapping or OOM.
*/
available = global_zone_page_state(NR_FREE_PAGES) - totalreserve_pages;
+ if (available < 0)
+ available = 0;
/*
* Not all the page cache can be freed, otherwise the system will
--
2.17.1
On Tue, Jan 03, 2023 at 04:28:07PM +0900, Jaewon Kim wrote:
> The totalreserve_pages could be higher than the free because of
> watermark high or watermark boost. Handle this situation and fix it to 0
> free size.
>
> Signed-off-by: Jaewon Kim <[email protected]>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 218b28ee49ed..e510ae83d5f3 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5948,6 +5948,8 @@ long si_mem_available(void)
> * without causing swapping or OOM.
> */
> available = global_zone_page_state(NR_FREE_PAGES) - totalreserve_pages;
> + if (available < 0)
> + available = 0;
>
> /*
> * Not all the page cache can be freed, otherwise the system will
> --
> 2.17.1
>
We already reset to zero at the end of the function, wouldn't resetting to zero
here potentially skew the result?
>--------- Original Message ---------
>Sender : Lorenzo Stoakes?<[email protected]>
>Date : 2023-01-03 16:35 (GMT+9)
>Title : Re: [PATCH] page_alloc: avoid the negative free for meminfo available
>?
>On Tue, Jan 03, 2023 at 04:28:07PM +0900, Jaewon Kim wrote:
>> The totalreserve_pages could be higher than the free because of
>> watermark high or watermark boost. Handle this situation and fix it to 0
>> free size.
>>
>> Signed-off-by: Jaewon Kim <[email protected]>
>> ---
>> ?mm/page_alloc.c | 2 ++
>> ?1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 218b28ee49ed..e510ae83d5f3 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5948,6 +5948,8 @@ long si_mem_available(void)
>> ? ? ? ? ? * without causing swapping or OOM.
>> ? ? ? ? ? */
>> ? ? ? ? ?available = global_zone_page_state(NR_FREE_PAGES) - totalreserve_pages;
>> + ? ? ? ?if (available < 0)
>> + ? ? ? ? ? ? ? ?available = 0;
>>
>> ? ? ? ? ?/*
>> ? ? ? ? ? * Not all the page cache can be freed, otherwise the system will
>> --
>> 2.17.1
>>
>
>We already reset to zero at the end of the function, wouldn't resetting to zero
>here potentially skew the result?
>
Hello
I did not mean the negative of the final available, we should account the actual size
by removing some improper portion of it. The free should be not negative in that perspective.
If negative, other parts like pagecache an reclailable would be decreased.
Actually pagecache and reclaimable are caculated with min, so I think reseting to zero
at the end the function is not necessary.
br
On Tue 03-01-23 16:28:07, Jaewon Kim wrote:
> The totalreserve_pages could be higher than the free because of
> watermark high or watermark boost. Handle this situation and fix it to 0
> free size.
What is the actual problem you are trying to address by this change?
> Signed-off-by: Jaewon Kim <[email protected]>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 218b28ee49ed..e510ae83d5f3 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5948,6 +5948,8 @@ long si_mem_available(void)
> * without causing swapping or OOM.
> */
> available = global_zone_page_state(NR_FREE_PAGES) - totalreserve_pages;
> + if (available < 0)
> + available = 0;
>
> /*
> * Not all the page cache can be freed, otherwise the system will
> --
> 2.17.1
--
Michal Hocko
SUSE Labs
>On Tue 03-01-23 16:28:07, Jaewon Kim wrote:
>> The totalreserve_pages could be higher than the free because of
>> watermark high or watermark boost. Handle this situation and fix it to 0
>> free size.
>
>What is the actual problem you are trying to address by this change?
Hello
As described on the original commit,
34e431b0ae39 /proc/meminfo: provide estimated available memory
mm is tring to provide the avaiable memory to user space.
But if free is negative, the available memory shown to userspace
would be shown smaller thatn the actual available size. The userspace
may do unwanted memory shrinking actions like process kills.
I think the logic sholud account the positive size only.
BR
>
>> Signed-off-by: Jaewon Kim <[email protected]>
>> ---
>> mm/page_alloc.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 218b28ee49ed..e510ae83d5f3 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5948,6 +5948,8 @@ long si_mem_available(void)
>> * without causing swapping or OOM.
>> */
>> available = global_zone_page_state(NR_FREE_PAGES) - totalreserve_pages;
>> + if (available < 0)
>> + available = 0;
>>
>> /*
>> * Not all the page cache can be freed, otherwise the system will
>> --
>> 2.17.1
>
>--
>Michal Hocko
>SUSE Labs
--------- Original Message ---------
Sender : Michal Hocko <[email protected]>
Date : 2023-01-03 17:03 (GMT+9)
Title : Re: [PATCH] page_alloc: avoid the negative free for meminfo available
On Tue 03-01-23 16:28:07, Jaewon Kim wrote:
> The totalreserve_pages could be higher than the free because of
> watermark high or watermark boost. Handle this situation and fix it to 0
> free size.
What is the actual problem you are trying to address by this change?
> Signed-off-by: Jaewon Kim <[email protected]>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 218b28ee49ed..e510ae83d5f3 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5948,6 +5948,8 @@ long si_mem_available(void)
> * without causing swapping or OOM.
> */
> available = global_zone_page_state(NR_FREE_PAGES) - totalreserve_pages;
> + if (available < 0)
> + available = 0;
>
> /*
> * Not all the page cache can be freed, otherwise the system will
> --
> 2.17.1
--
Michal Hocko
SUSE Labs