From: Linus Torvalds <torvalds@linux-foundation.org>
To: Konstantin Khlebnikov <koct9i@gmail.com>
Cc: linux-mm <linux-mm@kvack.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Cyrill Gorcunov <gorcunov@openvz.org>,
Christian Borntraeger <borntraeger@de.ibm.com>
Subject: Re: [PATCH] mm: enable RLIMIT_DATA by default with workaround for valgrind
Date: Sun, 24 Apr 2016 11:49:20 -0700 [thread overview]
Message-ID: <CA+55aFzPY23bL7oRaH8=C=CQ5egcWCEwieD5rhm5xV=Rv7T7RQ@mail.gmail.com> (raw)
In-Reply-To: <146148524340.530.2185181436065386014.stgit@zurg>
On Sun, Apr 24, 2016 at 1:07 AM, Konstantin Khlebnikov <koct9i@gmail.com> 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.
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@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-04-24 18:49 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-24 8:07 Konstantin Khlebnikov
2016-04-24 13:22 ` Cyrill Gorcunov
2016-04-24 18:49 ` Linus Torvalds [this message]
2016-04-24 20:40 ` Konstantin Khlebnikov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CA+55aFzPY23bL7oRaH8=C=CQ5egcWCEwieD5rhm5xV=Rv7T7RQ@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=akpm@linux-foundation.org \
--cc=borntraeger@de.ibm.com \
--cc=gorcunov@openvz.org \
--cc=koct9i@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox