linux-kernel - Re: [PATCH v1] ringbuffer: Don't choose the process with adj equal OOM_SCORE_ADJ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Apr 2018 14:00:36 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Joel Fernandes <joelaf@...gle.com>
Cc:     Michal Hocko <mhocko@...nel.org>,
        Zhaoyang Huang <huangzhaoyang@...il.com>,
        Ingo Molnar <mingo@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] ringbuffer: Don't choose the process with adj equal
 OOM_SCORE_ADJ_MIN

On Tue, 10 Apr 2018 09:45:54 -0700
Joel Fernandes <joelaf@...gle.com> wrote:

> > diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
> > index a0233edc0718..807e2bcb21b3 100644
> > --- a/include/linux/ring_buffer.h
> > +++ b/include/linux/ring_buffer.h
> > @@ -106,7 +106,8 @@ __poll_t ring_buffer_poll_wait(struct ring_buffer *buffer, int cpu,
> >
> >  void ring_buffer_free(struct ring_buffer *buffer);
> >
> > -int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size, int cpu);
> > +int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size,
> > +                       int cpu, int rbflags);
> >
> >  void ring_buffer_change_overwrite(struct ring_buffer *buffer, int val);
> >
> > @@ -201,6 +202,7 @@ int ring_buffer_print_page_header(struct trace_seq *s);
> >
> >  enum ring_buffer_flags {
> >         RB_FL_OVERWRITE         = 1 << 0,
> > +       RB_FL_NO_RECLAIM        = 1 << 1,  
> 
> But the thing is, set_oom_origin doesn't seem to be doing the
> desirable thing every time anyway as per my tests last week [1] and
> the si_mem_available check alone seems to be working fine for me (and
> also Zhaoyang as he mentioned).

But did you try it with just plain GFP_KERNEL, and not RETRY_MAYFAIL.
My tests would always trigger the allocating task without the
RETRY_MAYFAIL, but with RETRY_MAYFAIL it would sometimes take out other
tasks.

> 
> Since the problem Zhaoyang is now referring to is caused because of
> calling set_oom_origin in the first place, can we not just drop that
> patch and avoid adding more complexity?

Actually, I'm thinking of dropping the MAYFAIL part. It really should
be the one targeted if you are extending the ring buffer.

I could add two loops. One that does NORETRY without the oom origin,
and if it succeeds, its fine. But if it requires reclaim, it will then
set oom_origin and go harder (where it should be the one targeted).

But that may be pointless, because if NORETRY succeeds, there's not
really any likelihood of oom triggering in the first place.

> 
> IMHO I feel like for things like RB memory allocation, we shouldn't
> add a knob if we don't need to.

It was just a suggestion.

> 
> Also I think Zhaoyang is developing for Android too since he mentioned
> he ran CTS tests so we both have the same "usecase" but he can feel
> free to correct me if that's not the case ;)

I think if you are really worried with the task being killed by oom,
then I agree with Michal and just fork a process to do the allocation
for you.

-- Steve