linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mikulas Patocka <mpatocka@redhat.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Ondrej Kozina <okozina@redhat.com>,
	Jerome Marchand <jmarchan@redhat.com>,
	Stanislav Kozina <skozina@redhat.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	dm-devel@redhat.com
Subject: Re: System freezes after OOM
Date: Fri, 15 Jul 2016 08:11:22 -0400 (EDT)	[thread overview]
Message-ID: <alpine.LRH.2.02.1607150802380.5034@file01.intranet.prod.int.rdu2.redhat.com> (raw)
In-Reply-To: <20160715083510.GD11811@dhcp22.suse.cz>



On Fri, 15 Jul 2016, Michal Hocko wrote:

> On Thu 14-07-16 13:35:35, Mikulas Patocka wrote:
> > On Thu, 14 Jul 2016, Michal Hocko wrote:
> > > On Thu 14-07-16 10:00:16, Mikulas Patocka wrote:
> > > > But it needs other changes to honor the PF_LESS_THROTTLE flag:
> > > > 
> > > > static int current_may_throttle(void)
> > > > {
> > > >         return !(current->flags & PF_LESS_THROTTLE) ||
> > > >                 current->backing_dev_info == NULL ||
> > > >                 bdi_write_congested(current->backing_dev_info);
> > > > }
> > > > --- if you set PF_LESS_THROTTLE, current_may_throttle may still return 
> > > > true if one of the other conditions is met.
> > > 
> > > That is true but doesn't that mean that the device is congested and
> > > waiting a bit is the right thing to do?
> > 
> > You shouldn't really throttle mempool allocations at all. It's better to 
> > fail the allocation quickly and allocate from a mempool reserve than to 
> > wait 0.1 seconds in the reclaim path.
> 
> Well, but we do that already, no? The first allocation request is NOWAIT

The stacktraces showed that the kcryptd process was throttled when it 
tried to do mempool allocation. Mempool adds the __GFP_NORETRY flag to the 
allocation, but unfortunatelly, this flag doesn't prevent the allocator 
from throttling.

I say that the process doing mempool allocation shouldn't ever be 
throttled. Maybe add __GFP_NOTHROTTLE?

> and then we try to consume an object from the pool. We are re-adding
> __GFP_DIRECT_RECLAIM in case both fail. The point of throttling is to
> prevent from scanning through LRUs too quickly while we know that the
> bdi is congested.

> > dm-crypt can do approximatelly 100MB/s. That means that it processes 25k 
> > swap pages per second. If you wait in mempool_alloc, the allocation would 
> > be satisfied in 0.00004s. If you wait in the allocator's throttle 
> > function, you waste 0.1s.
> > 
> > 
> > It is also questionable if those 0.1 second sleeps are reasonable at all. 
> > SSDs with 100k IOPS are common - they can drain the request queue in much 
> > less time than 0.1 second. I think those hardcoded 0.1 second sleeps 
> > should be replaced with sleeps until the device stops being congested.
> 
> Well if we do not do throttle_vm_writeout then the only remaining
> writeout throttling for PF_LESS_THROTTLE is wait_iff_congested for
> the direct reclaim and that should wake up if the device stops being
> congested AFAIU.

I mean - a proper thing is to use active wakeup for the throttling, rather 
than retrying every 0.1 second. Polling for some condition is generally 
bad idea.

If there are too many pages under writeback, you should sleep on a wait 
queue. When the number of pages under writeback drops, wake up the wait 
queue.

Mikulas

> -- 
> Michal Hocko
> SUSE Labs
> 

  reply	other threads:[~2016-07-15 12:11 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <57837CEE.1010609@redhat.com>
     [not found] ` <f80dc690-7e71-26b2-59a2-5a1557d26713@redhat.com>
     [not found]   ` <9be09452-de7f-d8be-fd5d-4a80d1cd1ba3@redhat.com>
2016-07-11 15:43     ` System freezes after OOM Mikulas Patocka
2016-07-12  6:49       ` Michal Hocko
2016-07-12 23:44         ` Mikulas Patocka
2016-07-13  8:35           ` Jerome Marchand
2016-07-13 11:14             ` Michal Hocko
2016-07-13 14:21               ` Mikulas Patocka
2016-07-13 11:10           ` Michal Hocko
2016-07-13 12:50             ` Michal Hocko
2016-07-13 13:44               ` Milan Broz
2016-07-13 15:21                 ` Mikulas Patocka
2016-07-14  9:09                   ` Michal Hocko
2016-07-14  9:46                     ` Milan Broz
2016-07-13 15:02             ` Mikulas Patocka
2016-07-14 10:51               ` [dm-devel] " Ondrej Kozina
2016-07-14 12:51               ` Michal Hocko
2016-07-14 14:00                 ` Mikulas Patocka
2016-07-14 14:59                   ` Michal Hocko
2016-07-14 15:25                     ` Ondrej Kozina
2016-07-14 17:35                     ` Mikulas Patocka
2016-07-15  8:35                       ` Michal Hocko
2016-07-15 12:11                         ` Mikulas Patocka [this message]
2016-07-15 12:22                           ` Michal Hocko
2016-07-15 17:02                             ` Mikulas Patocka
2016-07-18  7:22                               ` Michal Hocko
2016-07-14 14:08                 ` Ondrej Kozina
2016-07-14 15:31                   ` Michal Hocko
2016-07-14 17:07                     ` Ondrej Kozina
2016-07-14 17:36                       ` Michal Hocko
2016-07-14 17:39                         ` Michal Hocko
2016-07-15 11:42                       ` Tetsuo Handa
2016-07-13 13:19           ` Tetsuo Handa
2016-07-13 13:39             ` Michal Hocko
2016-07-13 14:18               ` Mikulas Patocka
2016-07-13 14:56                 ` Michal Hocko
2016-07-13 15:11                   ` Mikulas Patocka
2016-07-13 23:53                     ` David Rientjes
2016-07-14 11:01                       ` Tetsuo Handa
2016-07-14 12:29                         ` Mikulas Patocka
2016-07-14 20:26                         ` David Rientjes
2016-07-14 21:40                           ` Tetsuo Handa
2016-07-14 22:04                             ` David Rientjes
2016-07-15 11:25                           ` Mikulas Patocka
2016-07-15 21:21                             ` David Rientjes
2016-07-14 12:27                       ` Mikulas Patocka
2016-07-14 20:22                         ` David Rientjes
2016-07-15 11:21                           ` Mikulas Patocka
2016-07-15 21:25                             ` David Rientjes
2016-07-15 21:39                               ` Mikulas Patocka
2016-07-15 21:58                                 ` David Rientjes
2016-07-15 23:53                                   ` Mikulas Patocka
2016-07-18 15:14                             ` Johannes Weiner
2016-07-14 15:29                       ` Michal Hocko
2016-07-14 20:38                         ` David Rientjes
2016-07-15  7:22                           ` Michal Hocko
2016-07-15  8:23                             ` Michal Hocko
2016-07-15 12:00                             ` Mikulas Patocka
2016-07-15 21:47                             ` David Rientjes
2016-07-18  7:39                               ` Michal Hocko
2016-07-18 21:03                                 ` David Rientjes
2016-07-14  0:01             ` David Rientjes

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=alpine.LRH.2.02.1607150802380.5034@file01.intranet.prod.int.rdu2.redhat.com \
    --to=mpatocka@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=jmarchan@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=okozina@redhat.com \
    --cc=skozina@redhat.com \
    /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;
as well as URLs for NNTP newsgroup(s).