Thread (232 messages) 232 messages, 19 authors, 2018-02-08

Re: [PATCH 4.14 108/159] kvm, mm: account kvm related kmem slabs to kmemcg

From: <hidden>
Date: 2017-12-22 18:07:55
Also in: lkml

On Fri, Dec 22, 2017 at 06:56:16PM +0100, Michal Hocko wrote:
On Fri 22-12-17 17:40:10, Sasha Levin wrote:
quoted
On Fri, Dec 22, 2017 at 02:06:07PM +0100, Michal Hocko wrote:
quoted
On Fri 22-12-17 13:41:22, Greg KH wrote:
quoted
On Fri, Dec 22, 2017 at 10:34:07AM +0100, Michal Hocko wrote:
quoted
On Fri 22-12-17 09:46:33, Greg KH wrote:
quoted
4.14-stable review patch.  If anyone has any objections, please let me know.

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

From: Shakeel Butt <redacted>


[ Upstream commit 46bea48ac241fe0b413805952dda74dd0c09ba8b ]

The kvm slabs can consume a significant amount of system memory
and indeed in our production environment we have observed that
a lot of machines are spending significant amount of memory that
can not be left as system memory overhead. Also the allocations
from these slabs can be triggered directly by user space applications
which has access to kvm and thus a buggy application can leak
such memory. So, these caches should be accounted to kmemcg.

Signed-off-by: Shakeel Butt <redacted>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sasha Levin <redacted>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The patch is not marked for stable, neither it fixes an existing bug.
It is a nice to have thing for sure but I am wondering how this got
through stable-filter.
Sasha picked it out, and it seemed like a sane thing to backport.  If
you think it's not worthy, I'll gladly drop it, but it seemed like such
a simple bugfix to include.
It is not that I would have some specific concerns about this particular
patch. It is more of a worry about the overal process. I thought that
_any_ patch backported to the stable tree would require a specific bug
to be fixed or in exceptional cases a performance issue. I have
experienced this pushback myself when trying to push "no real bug report
but better to have this plugged" patches.

So something has apparently changed in the process, I just haven't
noticed it. I am worried this might lead to more regression in future.
Not that my worry counts all that much as I am not a stable kernel user
though. So this is just my 2c worth of worry.
The way I see it is that stable commits are supposed to fix a bug that
a user can hit/exploit, it doesn't have to have an actual user
complaining about it.

For this particular commit, the way I read it is that a user can avoid
his kmemcg limits (maybe maliciously), which would qualify as an
actual bug we want to get fixed.
How are you going to judge all the possible relations to other
subsystems? I mean there is a good reason maintainers mark patches for
stable trees. How do you want to competently decide this for them? Can
you do that for all subsystems?

I do not want to underestimate your judgment or misinterpret your
process here but I _believe_ that picking patches based on the changelog
without a deep understanding of the subsystem is really risky. We do
not really have to go a long way to see that. Just look at other patch
in this very thread [1]. But maybe our our understanding of the stable
trees are different.
I don't try and override maintainers, I mostly try to get fixes out
of subsystems where maintainers/authors partially (or just don't)
mark their commits for stable.

These patches also go through a much longer review process than
commits that are marked for stable (there are at least 3 emails issued
for each such commit, and at least 1 week (usually much more) is
given for reviews).

-- 

Thanks,
Sasha
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help