Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1315240 > unrolled thread

Re: [PATCH RFC] mm: Rework virtual memory accounting

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH RFC] mm: Rework virtual memory accounting Christian Borntraeger <borntraeger@de.ibm.com> - 2016-01-22 20:50 +0100
    Re: [PATCH RFC] mm: Rework virtual memory accounting Andrew Morton <akpm@linux-foundation.org> - 2016-01-22 21:30 +0100
    Re: [PATCH RFC] mm: Rework virtual memory accounting Cyrill Gorcunov <gorcunov@gmail.com> - 2016-01-22 21:30 +0100
    Re: [PATCH RFC] mm: Rework virtual memory accounting Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-22 21:40 +0100

#1315240 — Re: [PATCH RFC] mm: Rework virtual memory accounting

On 12/28/2015 11:22 PM, Linus Torvalds wrote:
> On Mon, Dec 28, 2015 at 1:10 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>> Really sorry for delays. Konstantin, I slightly updated the
>> changelog (to point where problem came from). Linus are you
>> fine with accounting not only anonymous memory in VmData?
> 
> The patch looks ok to me. I guess if somebody relies on old behavior
> we may have to tweak it a bit, but on the whole this looks sane and
> I'd be happy to merge it in the 4.5 merge window (and maybe even have
> it marked for stable if it works out)
> 

Just want to mention that this patch breaks older versions of valgrind 
(including the current release)
https://bugs.kde.org/show_bug.cgi?id=357833
It is fixed in trunk (and even triggered some good cleanups, so the valgrind
developers do NOT want it to get reverted). Rawhide already has the valgrind
fix, others might not, so if we consider this for stable, things might break
here and there, but in general this looks like a good cleanup.

Christian

[toc] | [next] | [standalone]

#1315253

On Fri, 22 Jan 2016 20:42:11 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 12/28/2015 11:22 PM, Linus Torvalds wrote:
> > On Mon, Dec 28, 2015 at 1:10 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> >> Really sorry for delays. Konstantin, I slightly updated the
> >> changelog (to point where problem came from). Linus are you
> >> fine with accounting not only anonymous memory in VmData?
> > 
> > The patch looks ok to me. I guess if somebody relies on old behavior
> > we may have to tweak it a bit, but on the whole this looks sane and
> > I'd be happy to merge it in the 4.5 merge window (and maybe even have
> > it marked for stable if it works out)
> > 
> 
> Just want to mention that this patch breaks older versions of valgrind 
> (including the current release)
> https://bugs.kde.org/show_bug.cgi?id=357833
> It is fixed in trunk (and even triggered some good cleanups, so the valgrind
> developers do NOT want it to get reverted). Rawhide already has the valgrind
> fix, others might not, so if we consider this for stable, things might break
> here and there, but in general this looks like a good cleanup.
> 

OK, thanks - that sounds reasonable, although a bit worrisome - what
other userspace was affected?  In some cases people won't find out for
years...

84638335900f199 ("mm: rework virtual memory accounting") did not have
the cc:stable tag so it should avoid the -stable dragnet.

[toc] | [prev] | [next] | [standalone]

#1315255

On Fri, Jan 22, 2016 at 08:42:11PM +0100, Christian Borntraeger wrote:
> On 12/28/2015 11:22 PM, Linus Torvalds wrote:
> > On Mon, Dec 28, 2015 at 1:10 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> >> Really sorry for delays. Konstantin, I slightly updated the
> >> changelog (to point where problem came from). Linus are you
> >> fine with accounting not only anonymous memory in VmData?
> > 
> > The patch looks ok to me. I guess if somebody relies on old behavior
> > we may have to tweak it a bit, but on the whole this looks sane and
> > I'd be happy to merge it in the 4.5 merge window (and maybe even have
> > it marked for stable if it works out)
> > 
> 
> Just want to mention that this patch breaks older versions of valgrind 
> (including the current release)
> https://bugs.kde.org/show_bug.cgi?id=357833
> It is fixed in trunk (and even triggered some good cleanups, so the valgrind
> developers do NOT want it to get reverted). Rawhide already has the valgrind
> fix, others might not, so if we consider this for stable, things might break
> here and there, but in general this looks like a good cleanup.
> 
> Christian

Thanks a huge for the report, Christian. I think this won't go for stable
for now, lets see if there are other tools which do the same trick setting
up zero to rlimit data. If indeed this would make more problems than solve
it, we might need to find a way for backward compatibility.

[toc] | [prev] | [next] | [standalone]

#1315256

On Fri, Jan 22, 2016 at 11:42 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
>
> Just want to mention that this patch breaks older versions of valgrind
> (including the current release)
> https://bugs.kde.org/show_bug.cgi?id=357833

Ugh. Looks like valgrind is doing something that fundamentally can't
be "tweaked" around in the algorithm. Setting the data limit to zero
will never work with any model that starts accounting any mmap, so we
can't just tweak things a bit..

> It is fixed in trunk (and even triggered some good cleanups, so the valgrind
> developers do NOT want it to get reverted).

Hmm. If we start getting complaints from users, I suspect we'll just
have to revert. The fact that the valgrind developers are ok with the
change doesn't much matter - all that matters is whether users are ok
with it.

The only saving grace is that valgrind is fairly specialized, so it's
not like it breaks some core workflow. But I could easily see people
who run valgrind as part of some regression suite having their
day-to-day work broken.

So I'll let it slide for now, but if I start seeing complaints, I
think we'll just have to revert and wait for fixed valgrind versions
to actually percolate out to people and re-do it later.. (The
"percolate out to people" tends to take a _loong_ time, though).

                 Linus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web