Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] mm: mmap: show vm_unmapped_area error log
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     walken@google.com, bp@suse.de, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, jaewon31.kim@gmail.com
From:   Jaewon Kim <jaewon31.kim@samsung.com>
Message-ID: <5E61EAB6.5080609@samsung.com>
Date:   Fri, 6 Mar 2020 15:16:22 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
        Thunderbird/38.7.2
MIME-Version: 1.0
In-Reply-To: <20200305202443.8de3598558336b1d75afbba7@linux-foundation.org>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <CGME20200304030211epcas1p4da8cb569947aefb3aad1da039aaabce4@epcas1p4.samsung.com>
        <20200304030206.1706-1-jaewon31.kim@samsung.com>
        <5E605749.9050509@samsung.com>
        <20200305202443.8de3598558336b1d75afbba7@linux-foundation.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2020년 03월 06일 13:24, Andrew Morton wrote:
> On Thu, 5 Mar 2020 10:35:05 +0900 Jaewon Kim <jaewon31.kim@samsung.com> wrote:
>
>> Hello
>>
>> sorry for build warning.
>> I've changed %d to %ld reported by kbuild.
>> Let me attach full patch again below.
>> --------------------------------------------------
>>
>>
>> Even on 64 bit kernel, the mmap failure can happen for a 32 bit task.
>> Virtual memory space shortage of a task on mmap is reported to userspace
>> as -ENOMEM. It can be confused as physical memory shortage of overall
>> system.
>>
>> The vm_unmapped_area can be called to by some drivers or other kernel
>> core system like filesystem. It can be hard to know which code layer
>> returns the -ENOMEM.
>>
>> Print error log of vm_unmapped_area with rate limited. Without rate
>> limited, soft lockup ocurrs on infinite mmap sytem call.
>>
>> i.e.)
>> <3>[  576.024088]  [6:  mmap_infinite:14251] mmap: vm_unmapped_area err:-12 total_vm:0xfee08 flags:0x1 len:0xa00000 low:0x8000 high:0xf3f63000
>>
> hm, I suppose that could be useful.  Although the choice of which info
> to display could be a source of dispute.  Why did you choose this info
> and omit other things?  Perhaps a stack trace could also be useful?
I thought info->align_mask, info->align_offset are not important. But I added them too now.
And I think whole stack trace is not essential. In my opinion, higher layer like userspace mmap or other driver
calling to vm_unmapped_area also should report the error if they need.
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -2379,10 +2379,19 @@ extern unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info);
>>  static inline unsigned long
>>  vm_unmapped_area(struct vm_unmapped_area_info *info)
>>  {
>> +    unsigned long addr;
>> +
>>      if (info->flags & VM_UNMAPPED_AREA_TOPDOWN)
>> -        return unmapped_area_topdown(info);
>> +        addr = unmapped_area_topdown(info);
>>      else
>> -        return unmapped_area(info);
>> +        addr = unmapped_area(info);
>> +
>> +    if (IS_ERR_VALUE(addr) && printk_ratelimit()) {
> Please avoid using printk_ratelimit().  See the comment at the
> printk_ratelimit() definition site:
Thank you for your comment. I changed printk_ratelimit to pr_warn_ratelimited.
To use pr_warn_ratelimited I included <linux/ratelimit.h>
>
> /*
>  * Please don't use printk_ratelimit(), because it shares ratelimiting state
>  * with all other unrelated printk_ratelimit() callsites.  Instead use
>  * printk_ratelimited() or plain old __ratelimit().
>  */
>
>> +        pr_err("%s err:%ld total_vm:0x%lx flags:0x%lx len:0x%lx low:0x%lx high:0x%lx\n",
>> +            __func__, addr, current->mm->total_vm, info->flags,
>> +            info->length, info->low_limit, info->high_limit);
>> +    }
>> +    return addr;
>>  }
>
>

Let me attach changed whole patch below.

-------------------

Subject: [PATCH] mm: mmap: show vm_unmapped_area error log

Even on 64 bit kernel, the mmap failure can happen for a 32 bit task.
Virtual memory space shortage of a task on mmap is reported to userspace
as -ENOMEM. It can be confused as physical memory shortage of overall
system.

The vm_unmapped_area can be called to by some drivers or other kernel
core system like filesystem. It can be hard to know which code layer
returns the -ENOMEM.

Print error log of vm_unmapped_area with rate limited. Without rate
limited, soft lockup ocurrs on infinite mmap sytem call.

i.e.)
<4>[   68.556470]  [2:  mmap_infinite:12363] mmap: vm_unmapped_area err:-12 total_vm:0xf4c08 flags:0x1 len:0x100000 low:0x8000 high:0xf4583000 mask:0x0 offset:0x0

Fixed type mismatching on previous patch which is reported by kbuild.
Reported-by: kbuild test robot <lkp@intel.com>

Signed-off-by: Jaewon Kim <jaewon31.kim@samsung.com>
---
 include/linux/mm.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 52269e56c514..114055d70752 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -27,6 +27,7 @@
 #include <linux/memremap.h>
 #include <linux/overflow.h>
 #include <linux/sizes.h>
+#include <linux/ratelimit.h>
 
 struct mempolicy;
 struct anon_vma;
@@ -2379,10 +2380,20 @@ extern unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info);
 static inline unsigned long
 vm_unmapped_area(struct vm_unmapped_area_info *info)
 {
+    unsigned long addr;
+
     if (info->flags & VM_UNMAPPED_AREA_TOPDOWN)
-        return unmapped_area_topdown(info);
+        addr = unmapped_area_topdown(info);
     else
-        return unmapped_area(info);
+        addr = unmapped_area(info);
+
+    if (IS_ERR_VALUE(addr)) {
+        pr_warn_ratelimited("%s err:%ld total_vm:0x%lx flags:0x%lx len:0x%lx low:0x%lx high:0x%lx mask:0x%lx offset:0x%lx\n",
+            __func__, addr, current->mm->total_vm, info->flags,
+            info->length, info->low_limit, info->high_limit,
+            info->align_mask, info->align_offset);
+    }
+    return addr;
 }