Re: [PATCH] mm/util.c: Added page count to __vm_enough_memory failure warning

[David Hildenbrand <david@redhat.com>]

On 21.02.24 17:02, Matthew Cassell wrote:
> Commit 44b414c8715c5dcf53288 ("mm/util.c: add warning if __vm_enough_memory
> fails") adds debug information which gives the process id and executable name
> should __vm_enough_memory() fail. Adding the number of pages to the failure
> message would benefit application developers and system administrators in
> debugging overambitious memory requests by providing a point of reference to
> the amount of memory causing __vm_enough_memory() to fail.
> 
> 1. Set appropriate kernel tunable to reach code path for failure
>     message:
> 
> 	# echo 2 > /proc/sys/vm/overcommit_memory
> 
> 2. Test program to generate failure - requests 1 gibibyte per iteration:
> 
> 	#include <stdlib.h>
> 	#include <stdio.h>
> 
> 	int main(int argc, char **argv) {
> 		for(;;) {
> 			if(malloc(1<<30) == NULL)
> 				break;
> 
> 			printf("allocated 1 GiB\n");
> 		}
> 
> 		return 0;
> 	}
> 
> 3. Output:
> 
> 	Before:
> 
> 	__vm_enough_memory: pid: 1218, comm: a.out, not enough
> memory for the allocation
> 
> 	After:
> 
> 	__vm_enough_memory: pid: 1141, comm: a.out, pages: 262145, not
> enough memory for the allocation
> 
> Signed-off-by: Matthew Cassell <mcassell411@gmail.com>
> ---
>   mm/util.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/util.c b/mm/util.c
> index 5a6a9802583b..c0afb56f16ea 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -976,8 +976,8 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
>   	if (percpu_counter_read_positive(&vm_committed_as) < allowed)
>   		return 0;
>   error:
> -	pr_warn_ratelimited("%s: pid: %d, comm: %s, not enough memory for the allocation\n",
> -			    __func__, current->pid, current->comm);
> +	pr_warn_ratelimited("%s: pid: %d, comm: %s, pages: %ld, not enough memory for the allocation\n",
> +			    __func__, current->pid, current->comm, pages);
>   	vm_unacct_memory(pages);
>   
>   	return -ENOMEM;

I wonder if "bytes"/"kbytes" instead of pages would be more appropriate 
here.

Often, this will fail due to mmap() [where we pass a size from user 
space] and also "vm.overcommit_kbytes" is not in pages.

-- 
Cheers,

David / dhildenb