On 05/25/2018 01:16 PM, David Rientjes wrote: > When charging to a hugetlb_cgroup fails, alloc_huge_page() returns > ERR_PTR(-ENOSPC) which will cause VM_FAULT_SIGBUS to be returned to the > page fault handler. > > Instead, return the proper error code, ERR_PTR(-ENOMEM), so VM_FAULT_OOM > is handled correctly. This is consistent with failing mem cgroup charges > in the non-hugetlb fault path. Apologies for the late reply. I am not %100 sure we want to make this change. When hugetlb cgroup support was added by Aneesh, the intention was for the application to get SIGBUS. commit 2bc64a204697 https://lwn.net/Articles/499255/ Since the code has always caused SIGBUS when exceeding cgroup limit, there may be applications depending on this behavior. I would be especially concerned with HPC applications which were the original purpose for adding the feature. Perhaps, the original code should have returned ENOMEM to be consistent as in your patch. That does seem to be the more correct behavior. But, do we want to change behavior now (admittedly undocumented) and potentially break some application? I echo Michal's question about the reason for the change. If there is a real problem or issue to solve, that makes more of a case for making the change. If it is simply code/behavior cleanup for consistency then I would suggest not making the change, but rather documenting this as another hugetlbfs "special behavior". As a quick test, I added a shell to a hugetlb cgroup with 2 huge page limit and started a task using huge pages. Current behavior ---------------- ssh_to_dbg # sudo ./test_mmap 4 mapping 4 huge pages address 7f62bba00000 read (-) address 7f62bbc00000 read (-) Bus error ssh_to_dbg # Behavior with patch ------------------- ssh_to_dbg # sudo ./test_mmap 4 mapping 4 huge pages address 7f62bba00000 read (-) address 7f62bbc00000 read (-) Connection to dbg closed by remote host. Connection to dbf closed. OOM did kick in (lots of console/log output) and killed the shell as well. -- Mike Kravetz
