On Sun, Apr 24, 2016 at 9:49 PM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Sun, Apr 24, 2016 at 1:07 AM, Konstantin Khlebnikov <koct9i@xxxxxxxxx> wrote: >> >> This patch checks current usage also against rlim_max if rlim_cur is zero. >> Size of brk is still checked against rlim_cur, so this part is completely >> compatible - zero rlim_cur forbids brk() but allows private mmap(). > > The logic looks reasonable to me. My first reaction was that "but then > any process can set the limit to zero, and actually increase limits", > but witht he hard limit always being checked that's ok - the process > could have just set the soft limit to the hard limit instead. > > The only part I don't like in that patch is the disgusting line breaking. > > Breaking lines in the middle of a comparison is just nasty and wrong. > That code should have been written as > > if (rlimit(RLIMIT_DATA) != 0) > return false; > return mm->data_vm + npages <= rlimit_max(RLIMIT_DATA) >> PAGE_SHIFT; > > or something like that. Since you removed the pr_warn_once(), you > should remove ignore_rlimit_data too. > > Alternatively, if you want to keep ignore_rlimit_data, then you should > have kept the warning too. Making the actual rlimit data check an > inline helper function and having the ignore_rlimit_data check (and > printout) in the caller would make it pretty. Ok, I'll keep boot option and warning. This should make transition less painful. That data segment limit was almost useless for a long time and now it actually starts working, I'm sure there are a lot of strange configurations around it. And I want to keep stuff in one function - this simplifies revert =) > > Because breaking lines in the middle of an actual expression is just > completely wrong. It's much worse than having a long line. > > (The exception to that "middle of an expression" is breaking lines at > logical expression boundaries: things like adding up several > independent expressions, and having it be > > sum = a + > b + > c; > > or be something like > > if (a || > b || > c) > do_something(): > > where 'a', 'b' and 'c' are complex but fairly independent expressions). > > Linus -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>