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

Groups > linux.kernel > #1440722 > unrolled thread

Re: System freezes after OOM

Started byMikulas Patocka <mpatocka@redhat.com>
First post2016-07-11 17:50 +0200
Last post2016-07-14 02:10 +0200
Articles 20 on this page of 60 — 8 participants

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: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-11 17:50 +0200
    Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-12 08:50 +0200
      Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 01:50 +0200
        Re: System freezes after OOM Jerome Marchand <jmarchan@redhat.com> - 2016-07-13 10:40 +0200
          Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 13:20 +0200
            Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 16:30 +0200
        Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 13:20 +0200
          Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 15:00 +0200
            Re: System freezes after OOM Milan Broz <gmazyland@gmail.com> - 2016-07-13 15:50 +0200
              Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 17:40 +0200
                Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 11:10 +0200
                  Re: System freezes after OOM Milan Broz <gmazyland@gmail.com> - 2016-07-14 11:50 +0200
          Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 17:10 +0200
            Re: [dm-devel] System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 13:00 +0200
            Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 15:00 +0200
              Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 16:10 +0200
                Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 17:00 +0200
                  Re: System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 17:30 +0200
                  Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 19:40 +0200
                    Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 10:40 +0200
                      Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 14:20 +0200
                        Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 14:30 +0200
                          Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 19:10 +0200
                            Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-18 09:30 +0200
              Re: System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 17:30 +0200
                Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 17:40 +0200
                  Re: System freezes after OOM Ondrej Kozina <okozina@redhat.com> - 2016-07-14 19:10 +0200
                    Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 19:40 +0200
                      Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 19:40 +0200
                    Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-15 13:50 +0200
        Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-13 15:30 +0200
          Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 15:50 +0200
            Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 16:30 +0200
              Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-13 17:00 +0200
                Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-13 17:20 +0200
                  Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 02:00 +0200
                    Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-14 13:10 +0200
                      Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 14:30 +0200
                      Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 22:30 +0200
                        Re: System freezes after OOM Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-07-14 23:50 +0200
                          Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 00:10 +0200
                        Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 13:30 +0200
                          Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 23:30 +0200
                    Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-14 14:30 +0200
                      Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 22:30 +0200
                        Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 13:30 +0200
                          Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 23:30 +0200
                            Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 23:50 +0200
                              Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-16 00:00 +0200
                                Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-16 02:00 +0200
                          Re: System freezes after OOM Johannes Weiner <hannes@cmpxchg.org> - 2016-07-18 17:20 +0200
                    Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-14 17:30 +0200
                      Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 22:40 +0200
                        Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 09:30 +0200
                          Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-15 10:30 +0200
                          Re: System freezes after OOM Mikulas Patocka <mpatocka@redhat.com> - 2016-07-15 14:10 +0200
                          Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-15 23:50 +0200
                            Re: System freezes after OOM Michal Hocko <mhocko@kernel.org> - 2016-07-18 09:40 +0200
                              Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-18 23:10 +0200
          Re: System freezes after OOM David Rientjes <rientjes@google.com> - 2016-07-14 02:10 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1444240

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-07-15 14:20 +0200
Message-ID<rVag1-6Nq-1@gated-at.bofh.it>
In reply to#1444058

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
>

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

#1444253

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-15 14:30 +0200
Message-ID<rVapH-6QL-13@gated-at.bofh.it>
In reply to#1444240
On Fri 15-07-16 08:11:22, Mikulas Patocka wrote:
> 
> 
> 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.

Yes and in fact it shouldn't prevent any throttling. The flag merely
says that the allocation should give up rather than retry
reclaim/compaction again and again.

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

A specific gfp flag would be an option but we are slowly running out of
bit space there and I am not yet convinced PF_LESS_THROTTLE is
unsuitable.

> > 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.

I might be missing something but exactly this is what happens in
wait_iff_congested no? If the bdi doesn't see the congestion it wakes up
the reclaim context even before the timeout. Or are we talking past each
other?

-- 
Michal Hocko
SUSE Labs

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

#1444459

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-07-15 19:10 +0200
Message-ID<rVeMF-19a-15@gated-at.bofh.it>
In reply to#1444253

On Fri, 15 Jul 2016, Michal Hocko wrote:

> On Fri 15-07-16 08:11:22, Mikulas Patocka wrote:
> > 
> > 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.
> 
> Yes and in fact it shouldn't prevent any throttling. The flag merely
> says that the allocation should give up rather than retry
> reclaim/compaction again and again.
> 
> > I say that the process doing mempool allocation shouldn't ever be 
> > throttled. Maybe add __GFP_NOTHROTTLE?
> 
> A specific gfp flag would be an option but we are slowly running out of
> bit space there and I am not yet convinced PF_LESS_THROTTLE is
> unsuitable.

PF_LESS_THROTTLE will make it throttle less, but it doesn't eliminate 
throttling entirely. So, maybe add PF_NO_THROTTLE? But PF_* flags are also 
almost exhausted.

> I might be missing something but exactly this is what happens in
> wait_iff_congested no? If the bdi doesn't see the congestion it wakes up
> the reclaim context even before the timeout. Or are we talking past each
> other?

OK, I see that there is wait queue in congestion_wait. I didn't notice it 
before.

Mikulas

> -- 
> Michal Hocko
> SUSE Labs
>

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

#1445275

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-18 09:30 +0200
Message-ID<rWba2-3by-27@gated-at.bofh.it>
In reply to#1444459
On Fri 15-07-16 13:02:17, Mikulas Patocka wrote:
> 
> 
> On Fri, 15 Jul 2016, Michal Hocko wrote:
> 
> > On Fri 15-07-16 08:11:22, Mikulas Patocka wrote:
> > > 
> > > 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.
> > 
> > Yes and in fact it shouldn't prevent any throttling. The flag merely
> > says that the allocation should give up rather than retry
> > reclaim/compaction again and again.
> > 
> > > I say that the process doing mempool allocation shouldn't ever be 
> > > throttled. Maybe add __GFP_NOTHROTTLE?
> > 
> > A specific gfp flag would be an option but we are slowly running out of
> > bit space there and I am not yet convinced PF_LESS_THROTTLE is
> > unsuitable.
> 
> PF_LESS_THROTTLE will make it throttle less, but it doesn't eliminate 
> throttling entirely. So, maybe add PF_NO_THROTTLE? But PF_* flags are also 
> almost exhausted.

I am not really sure we can make anybody so special to not throttle at all.
Seeing a congested backig device sounds like a reasonable compromise.
Besides that it seems that we do not really need to eliminate
wait_iff_congested for dm to work properly again AFAIU. I plan to repost
both patch today after some more internal review. If we need to do more
changes I would suggest making them in separet patches.
-- 
Michal Hocko
SUSE Labs

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

#1443534

FromOndrej Kozina <okozina@redhat.com>
Date2016-07-14 17:30 +0200
Message-ID<rUQKm-2Yl-25@gated-at.bofh.it>
In reply to#1443431
On 07/14/2016 02:51 PM, Michal Hocko wrote:
> On Wed 13-07-16 11:02:15, Mikulas Patocka wrote:
>> On Wed, 13 Jul 2016, Michal Hocko wrote:
> [...]
>
> We are discussing several topics together so let's focus on this
> particlar thing for now
>
>>>> The kernel 4.7-rc almost deadlocks in another way. The machine got stuck
>>>> and the following stacktrace was obtained when swapping to dm-crypt.
>>>>
>>>> We can see that dm-crypt does a mempool allocation. But the mempool
>>>> allocation somehow falls into throttle_vm_writeout. There, it waits for
>>>> 0.1 seconds. So, as a result, the dm-crypt worker thread ends up
>>>> processing requests at an unusually slow rate of 10 requests per second
>>>> and it results in the machine being stuck (it would proabably recover if
>>>> we waited for extreme amount of time).
>>>
>>> Hmm, that throttling is there since ever basically. I do not see what
>>> would have changed that recently, but I haven't looked too close to be
>>> honest.
>>>
>>> I agree that throttling a flusher (which this worker definitely is)
>>> doesn't look like a correct thing to do. We have PF_LESS_THROTTLE for
>>> this kind of things. So maybe the right thing to do is to use this flag
>>> for the dm_crypt worker:
>>>
>>> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>>> index 4f3cb3554944..0b806810efab 100644
>>> --- a/drivers/md/dm-crypt.c
>>> +++ b/drivers/md/dm-crypt.c
>>> @@ -1392,11 +1392,14 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
>>>  static void kcryptd_crypt(struct work_struct *work)
>>>  {
>>>  	struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
>>> +	unsigned int pflags = current->flags;
>>>
>>> +	current->flags |= PF_LESS_THROTTLE;
>>>  	if (bio_data_dir(io->base_bio) == READ)
>>>  		kcryptd_crypt_read_convert(io);
>>>  	else
>>>  		kcryptd_crypt_write_convert(io);
>>> +	tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
>>>  }
>>>
>>>  static void kcryptd_queue_crypt(struct dm_crypt_io *io)
>>
>> ^^^ That fixes just one specific case - but there may be other threads
>> doing mempool allocations in the device mapper subsystem - and you would
>> need to mark all of them.
>
> Now that I am thinking about it some more. Are there any mempool users
> which would actually want to be throttled? I would expect mempool users
> are necessary to push IO through and throttle them sounds like a bad
> decision in the first place but there might be other mempool users which
> could cause issues. Anyway how about setting PF_LESS_THROTTLE
> unconditionally inside mempool_alloc? Something like the following:
>
> diff --git a/mm/mempool.c b/mm/mempool.c
> index 8f65464da5de..e21fb632983f 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -310,7 +310,8 @@ EXPORT_SYMBOL(mempool_resize);
>   */
>  void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>  {
> -	void *element;
> +	unsigned int pflags = current->flags;
> +	void *element = NULL;
>  	unsigned long flags;
>  	wait_queue_t wait;
>  	gfp_t gfp_temp;
> @@ -327,6 +328,12 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>
>  	gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
>
> +	/*
> +	 * Make sure that the allocation doesn't get throttled during the
> +	 * reclaim
> +	 */
> +	if (gfpflags_allow_blocking(gfp_mask))
> +		current->flags |= PF_LESS_THROTTLE;
>  repeat_alloc:
>  	if (likely(pool->curr_nr)) {
>  		/*
> @@ -339,7 +346,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>
>  	element = pool->alloc(gfp_temp, pool->pool_data);
>  	if (likely(element != NULL))
> -		return element;
> +		goto out;
>
>  	spin_lock_irqsave(&pool->lock, flags);
>  	if (likely(pool->curr_nr)) {
> @@ -352,7 +359,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>  		 * for debugging.
>  		 */
>  		kmemleak_update_trace(element);
> -		return element;
> +		goto out;
>  	}
>
>  	/*
> @@ -369,7 +376,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>  	/* We must not sleep if !__GFP_DIRECT_RECLAIM */
>  	if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
>  		spin_unlock_irqrestore(&pool->lock, flags);
> -		return NULL;
> +		goto out;
>  	}
>
>  	/* Let's wait for someone else to return an element to @pool */
> @@ -386,6 +393,10 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
>
>  	finish_wait(&pool->wait, &wait);
>  	goto repeat_alloc;
> +out:
> +	if (gfpflags_allow_blocking(gfp_mask))
> +		tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> +	return element;
>  }
>  EXPORT_SYMBOL(mempool_alloc);
>
>

As Mikulas pointed out, this doesn't work. The system froze as well with 
the patch above. Will try to tweak the patch with Mikulas's suggestion...

O.

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

#1443545

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-14 17:40 +0200
Message-ID<rUQU1-31t-15@gated-at.bofh.it>
In reply to#1443534
On Thu 14-07-16 16:08:28, Ondrej Kozina wrote:
[...]
> As Mikulas pointed out, this doesn't work. The system froze as well with the
> patch above. Will try to tweak the patch with Mikulas's suggestion...

Thank you for testing! Do you happen to have traces of the frozen
processes? Does the flusher still gets throttled because the bias it
gets is not sufficient. Or does it get throttled at a different place?
-- 
Michal Hocko
SUSE Labs

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

#1443603

FromOndrej Kozina <okozina@redhat.com>
Date2016-07-14 19:10 +0200
Message-ID<rUSj7-41y-7@gated-at.bofh.it>
In reply to#1443545
On 07/14/2016 05:31 PM, Michal Hocko wrote:
> On Thu 14-07-16 16:08:28, Ondrej Kozina wrote:
> [...]
>> As Mikulas pointed out, this doesn't work. The system froze as well with the
>> patch above. Will try to tweak the patch with Mikulas's suggestion...
>
> Thank you for testing! Do you happen to have traces of the frozen
> processes? Does the flusher still gets throttled because the bias it
> gets is not sufficient. Or does it get throttled at a different place?
>

Sure. Here it is (including sysrq+t and sysrq+w output): 
https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/4.7.0-rc7+/1/4.7.0-rc7+.log

In a directory with the log there's also a patch the kernel was compiled 
with.

Ondra

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

#1443627

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-14 19:40 +0200
Message-ID<rUSMa-4cH-19@gated-at.bofh.it>
In reply to#1443603
On Thu 14-07-16 19:07:52, Ondrej Kozina wrote:
> On 07/14/2016 05:31 PM, Michal Hocko wrote:
> > On Thu 14-07-16 16:08:28, Ondrej Kozina wrote:
> > [...]
> > > As Mikulas pointed out, this doesn't work. The system froze as well with the
> > > patch above. Will try to tweak the patch with Mikulas's suggestion...
> > 
> > Thank you for testing! Do you happen to have traces of the frozen
> > processes? Does the flusher still gets throttled because the bias it
> > gets is not sufficient. Or does it get throttled at a different place?
> > 
> 
> Sure. Here it is (including sysrq+t and sysrq+w output): https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/4.7.0-rc7+/1/4.7.0-rc7+.log

Thanks a lot! This is helpful.
[  162.716376] active_anon:107874 inactive_anon:108176 isolated_anon:64
[  162.716376]  active_file:1086 inactive_file:1103 isolated_file:0
[  162.716376]  unevictable:0 dirty:0 writeback:69824 unstable:0
[  162.716376]  slab_reclaimable:3119 slab_unreclaimable:24124
[  162.716376]  mapped:2165 shmem:57 pagetables:1509 bounce:0
[  162.716376]  free:701 free_pcp:0 free_cma:0

No surprise that PF_LESS_THROTTLE didn't help. It gives some bias but
considering how many pages are under writeback it cannot possibly help
to prevent from sleeping in throttle_vm_writeout. I suppose adding
the following on top of the memalloc patch helps, right?
It is an alternative to what you were suggesting in other email but
it doesn't affect current_may_throttle paths which I would rather not
touch.
---
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 7fbb2d008078..a37661f1a11b 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1971,6 +1971,9 @@ void throttle_vm_writeout(gfp_t gfp_mask)
 	unsigned long background_thresh;
 	unsigned long dirty_thresh;
 
+	if (current->flags & PF_LESS_THROTTLE)
+		return;
+
         for ( ; ; ) {
 		global_dirty_limits(&background_thresh, &dirty_thresh);
 		dirty_thresh = hard_dirty_limit(&global_wb_domain, dirty_thresh);
-- 
Michal Hocko
SUSE Labs

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

#1443634

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-14 19:40 +0200
Message-ID<rUSMa-4cH-33@gated-at.bofh.it>
In reply to#1443627
On Thu 14-07-16 19:36:59, Michal Hocko wrote:
> On Thu 14-07-16 19:07:52, Ondrej Kozina wrote:
> > On 07/14/2016 05:31 PM, Michal Hocko wrote:
> > > On Thu 14-07-16 16:08:28, Ondrej Kozina wrote:
> > > [...]
> > > > As Mikulas pointed out, this doesn't work. The system froze as well with the
> > > > patch above. Will try to tweak the patch with Mikulas's suggestion...
> > > 
> > > Thank you for testing! Do you happen to have traces of the frozen
> > > processes? Does the flusher still gets throttled because the bias it
> > > gets is not sufficient. Or does it get throttled at a different place?
> > > 
> > 
> > Sure. Here it is (including sysrq+t and sysrq+w output): https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/4.7.0-rc7+/1/4.7.0-rc7+.log
> 
> Thanks a lot! This is helpful.
> [  162.716376] active_anon:107874 inactive_anon:108176 isolated_anon:64
> [  162.716376]  active_file:1086 inactive_file:1103 isolated_file:0
> [  162.716376]  unevictable:0 dirty:0 writeback:69824 unstable:0
> [  162.716376]  slab_reclaimable:3119 slab_unreclaimable:24124
> [  162.716376]  mapped:2165 shmem:57 pagetables:1509 bounce:0
> [  162.716376]  free:701 free_pcp:0 free_cma:0
> 
> No surprise that PF_LESS_THROTTLE didn't help. It gives some bias but
> considering how many pages are under writeback it cannot possibly help
> to prevent from sleeping in throttle_vm_writeout. I suppose adding
> the following on top of the memalloc patch helps, right?
> It is an alternative to what you were suggesting in other email but
> it doesn't affect current_may_throttle paths which I would rather not
> touch.

Just read the other email and your patch properly and this is basically
equivalent thing. I will think more about potential issues this might
cause and send a proper patch for wider review.

> ---
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 7fbb2d008078..a37661f1a11b 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -1971,6 +1971,9 @@ void throttle_vm_writeout(gfp_t gfp_mask)
>  	unsigned long background_thresh;
>  	unsigned long dirty_thresh;
>  
> +	if (current->flags & PF_LESS_THROTTLE)
> +		return;
> +
>          for ( ; ; ) {
>  		global_dirty_limits(&background_thresh, &dirty_thresh);
>  		dirty_thresh = hard_dirty_limit(&global_wb_domain, dirty_thresh);
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

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

#1444213

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2016-07-15 13:50 +0200
Message-ID<rV9N0-6nQ-23@gated-at.bofh.it>
In reply to#1443603
On 2016/07/15 2:07, Ondrej Kozina wrote:
> On 07/14/2016 05:31 PM, Michal Hocko wrote:
>> On Thu 14-07-16 16:08:28, Ondrej Kozina wrote:
>> [...]
>>> As Mikulas pointed out, this doesn't work. The system froze as well with the
>>> patch above. Will try to tweak the patch with Mikulas's suggestion...
>>
>> Thank you for testing! Do you happen to have traces of the frozen
>> processes? Does the flusher still gets throttled because the bias it
>> gets is not sufficient. Or does it get throttled at a different place?
>>
> 
> Sure. Here it is (including sysrq+t and sysrq+w output): https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/4.7.0-rc7+/1/4.7.0-rc7+.log
> 

Oh, this resembles another dm-crypt lockup problem reported last month.
( http://lkml.kernel.org/r/20160616212641.GA3308@sig21.net )

In Johannes's case, there are so many pending kcryptd_crypt work requests and
mempool_alloc() is waiting at throttle_vm_writeout() or shrink_inactive_list().

[ 2378.279029] kswapd0         D ffff88003744f538     0   766      2 0x00000000
[ 2378.286167]  ffff88003744f538 00ff88011b5ccd80 ffff88011b5d62d8 ffff88011ae58000
[ 2378.293628]  ffff880037450000 ffff880037450000 00000001000984f2 ffff88003744f570
[ 2378.301168]  ffff88011b5ccd80 ffff880037450000 ffff88003744f550 ffffffff81845cec
[ 2378.308674] Call Trace:
[ 2378.311154]  [<ffffffff81845cec>] schedule+0x8b/0xa3
[ 2378.316153]  [<ffffffff81849b5b>] schedule_timeout+0x20b/0x285
[ 2378.322028]  [<ffffffff810e6da6>] ? init_timer_key+0x112/0x112
[ 2378.327931]  [<ffffffff81845070>] io_schedule_timeout+0xa0/0x102
[ 2378.333960]  [<ffffffff81845070>] ? io_schedule_timeout+0xa0/0x102
[ 2378.340166]  [<ffffffff81162c2b>] mempool_alloc+0x123/0x154
[ 2378.345781]  [<ffffffff810bdd00>] ? wait_woken+0x72/0x72
[ 2378.351148]  [<ffffffff8133fdc1>] bio_alloc_bioset+0xe8/0x1d7
[ 2378.356910]  [<ffffffff816342ea>] alloc_tio+0x2d/0x47
[ 2378.361996]  [<ffffffff8163587e>] __split_and_process_bio+0x310/0x3a3
[ 2378.368470]  [<ffffffff81635e15>] dm_make_request+0xb5/0xe2
[ 2378.374078]  [<ffffffff81347ae7>] generic_make_request+0xcc/0x180
[ 2378.380206]  [<ffffffff81347c98>] submit_bio+0xfd/0x145
[ 2378.385482]  [<ffffffff81198948>] __swap_writepage+0x202/0x225
[ 2378.391349]  [<ffffffff810a5eeb>] ? preempt_count_sub+0xf0/0x100
[ 2378.397398]  [<ffffffff8184a5f7>] ? _raw_spin_unlock+0x31/0x44
[ 2378.403273]  [<ffffffff8119a903>] ? page_swapcount+0x45/0x4c
[ 2378.408984]  [<ffffffff811989a5>] swap_writepage+0x3a/0x3e
[ 2378.414530]  [<ffffffff811727ef>] pageout.isra.16+0x160/0x2a7
[ 2378.420320]  [<ffffffff81173a8f>] shrink_page_list+0x5a0/0x8c4
[ 2378.426197]  [<ffffffff81174489>] shrink_inactive_list+0x29e/0x4a1
[ 2378.432434]  [<ffffffff81174e8b>] shrink_zone_memcg+0x4c1/0x661
[ 2378.438406]  [<ffffffff81175107>] shrink_zone+0xdc/0x1e5
[ 2378.443742]  [<ffffffff81175107>] ? shrink_zone+0xdc/0x1e5
[ 2378.449238]  [<ffffffff8117628f>] kswapd+0x6df/0x814
[ 2378.454222]  [<ffffffff81175bb0>] ? mem_cgroup_shrink_node_zone+0x209/0x209
[ 2378.461196]  [<ffffffff8109f208>] kthread+0xff/0x107
[ 2378.466182]  [<ffffffff8184b1f2>] ret_from_fork+0x22/0x50
[ 2378.471631]  [<ffffffff8109f109>] ? kthread_create_on_node+0x1ea/0x1ea

[ 2378.769494] kworker/u8:4    D ffff8800c5dc3508     0  1592      2 0x00000000
[ 2378.776582] Workqueue: kcryptd kcryptd_crypt
[ 2378.780887]  ffff8800c5dc3508 00ff88011b7ccd80 ffff88011b7d62d8 ffff88011ae5a900
[ 2378.788399]  ffff88011a605200 ffff8800c5dc4000 00000001000983f7 ffff8800c5dc3540
[ 2378.795930]  ffff88011b7ccd80 0000000000000000 ffff8800c5dc3520 ffffffff81845cec
[ 2378.803408] Call Trace:
[ 2378.805879]  [<ffffffff81845cec>] schedule+0x8b/0xa3
[ 2378.810908]  [<ffffffff81849b5b>] schedule_timeout+0x20b/0x285
[ 2378.816783]  [<ffffffff810e6da6>] ? init_timer_key+0x112/0x112
[ 2378.822677]  [<ffffffff81845070>] io_schedule_timeout+0xa0/0x102
[ 2378.828716]  [<ffffffff81845070>] ? io_schedule_timeout+0xa0/0x102
[ 2378.834956]  [<ffffffff8117d5c0>] congestion_wait+0x84/0x160
[ 2378.840658]  [<ffffffff810bdd00>] ? wait_woken+0x72/0x72
[ 2378.845997]  [<ffffffff8116c32f>] throttle_vm_writeout+0x88/0xab
[ 2378.852036]  [<ffffffff81174fff>] shrink_zone_memcg+0x635/0x661
[ 2378.857982]  [<ffffffff81175107>] shrink_zone+0xdc/0x1e5
[ 2378.863309]  [<ffffffff81175107>] ? shrink_zone+0xdc/0x1e5
[ 2378.868832]  [<ffffffff811753b5>] do_try_to_free_pages+0x1a5/0x2c3
[ 2378.875028]  [<ffffffff811755f6>] try_to_free_pages+0x123/0x21f
[ 2378.880972]  [<ffffffff81168216>] __alloc_pages_nodemask+0x4c9/0x978
[ 2378.887385]  [<ffffffff8138027a>] ? debug_smp_processor_id+0x17/0x19
[ 2378.893782]  [<ffffffff8119fb2a>] new_slab+0xbc/0x3bb
[ 2378.898868]  [<ffffffff811a1acd>] ___slab_alloc.constprop.22+0x2fb/0x37b
[ 2378.905634]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2378.911659]  [<ffffffff8101f5ba>] ? sched_clock+0x9/0xd
[ 2378.916909]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2378.922325]  [<ffffffff810c6438>] ? __lock_acquire.isra.16+0x55e/0xb4c
[ 2378.928877]  [<ffffffff8101f5ba>] ? sched_clock+0x9/0xd
[ 2378.934138]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2378.939555]  [<ffffffff810c6438>] ? __lock_acquire.isra.16+0x55e/0xb4c
[ 2378.946125]  [<ffffffff811a1ba4>] __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2378.953289]  [<ffffffff811a1ba4>] ? __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2378.960630]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2378.966706]  [<ffffffff811a1c78>] kmem_cache_alloc+0xa0/0x1d6
[ 2378.972503]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2378.978567]  [<ffffffff81162a88>] mempool_alloc_slab+0x15/0x17
[ 2378.984426]  [<ffffffff81162b7a>] mempool_alloc+0x72/0x154
[ 2378.989930]  [<ffffffff810c4b45>] ? lockdep_init_map+0xc9/0x5a3
[ 2378.995866]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2379.001300]  [<ffffffff8133fdc1>] bio_alloc_bioset+0xe8/0x1d7
[ 2379.007107]  [<ffffffff81643127>] kcryptd_crypt+0x1ab/0x325
[ 2379.012704]  [<ffffffff810998fd>] ? process_one_work+0x1ad/0x4e2
[ 2379.018753]  [<ffffffff810999d3>] process_one_work+0x283/0x4e2
[ 2379.024629]  [<ffffffff810c502d>] ? put_lock_stats.isra.9+0xe/0x20
[ 2379.030851]  [<ffffffff8109a860>] worker_thread+0x285/0x370
[ 2379.036423]  [<ffffffff8109a5db>] ? rescuer_thread+0x2d1/0x2d1
[ 2379.042309]  [<ffffffff8109f208>] kthread+0xff/0x107
[ 2379.047310]  [<ffffffff8184b1f2>] ret_from_fork+0x22/0x50
[ 2379.052726]  [<ffffffff8109f109>] ? kthread_create_on_node+0x1ea/0x1ea

[ 2379.059328] kworker/u8:6    D ffff8800c5ec3508     0  1594      2 0x00000000
[ 2379.066468] Workqueue: kcryptd kcryptd_crypt
[ 2379.070808]  ffff8800c5ec3508 00ff88011b7ccd80 ffff88011b7d62d8 ffff88011ae5a900
[ 2379.078296]  ffff88003749a900 ffff8800c5ec4000 0000000100098467 ffff8800c5ec3540
[ 2379.085836]  ffff88011b7ccd80 0000000000000000 ffff8800c5ec3520 ffffffff81845cec
[ 2379.093315] Call Trace:
[ 2379.095776]  [<ffffffff81845cec>] schedule+0x8b/0xa3
[ 2379.100785]  [<ffffffff81849b5b>] schedule_timeout+0x20b/0x285
[ 2379.106627]  [<ffffffff810e6da6>] ? init_timer_key+0x112/0x112
[ 2379.112494]  [<ffffffff81845070>] io_schedule_timeout+0xa0/0x102
[ 2379.118524]  [<ffffffff81845070>] ? io_schedule_timeout+0xa0/0x102
[ 2379.124740]  [<ffffffff8117d5c0>] congestion_wait+0x84/0x160
[ 2379.130432]  [<ffffffff810bdd00>] ? wait_woken+0x72/0x72
[ 2379.135771]  [<ffffffff8116c32f>] throttle_vm_writeout+0x88/0xab
[ 2379.141839]  [<ffffffff81174fff>] shrink_zone_memcg+0x635/0x661
[ 2379.147810]  [<ffffffff81175107>] shrink_zone+0xdc/0x1e5
[ 2379.153155]  [<ffffffff81175107>] ? shrink_zone+0xdc/0x1e5
[ 2379.158651]  [<ffffffff811753b5>] do_try_to_free_pages+0x1a5/0x2c3
[ 2379.164881]  [<ffffffff811755f6>] try_to_free_pages+0x123/0x21f
[ 2379.170861]  [<ffffffff81168216>] __alloc_pages_nodemask+0x4c9/0x978
[ 2379.177292]  [<ffffffff811a1776>] ? get_partial_node.isra.19+0x353/0x3af
[ 2379.184026]  [<ffffffff8119fb2a>] new_slab+0xbc/0x3bb
[ 2379.189103]  [<ffffffff811a1acd>] ___slab_alloc.constprop.22+0x2fb/0x37b
[ 2379.195843]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2379.201895]  [<ffffffff81049da2>] ? glue_xts_crypt_128bit+0x1a6/0x1d8
[ 2379.208357]  [<ffffffff8101f5ba>] ? sched_clock+0x9/0xd
[ 2379.213610]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2379.219050]  [<ffffffff810c6438>] ? __lock_acquire.isra.16+0x55e/0xb4c
[ 2379.225596]  [<ffffffff811a1ba4>] __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2379.232769]  [<ffffffff811a1ba4>] ? __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2379.240143]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2379.246177]  [<ffffffff811a1c78>] kmem_cache_alloc+0xa0/0x1d6
[ 2379.251957]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2379.258024]  [<ffffffff81162a88>] mempool_alloc_slab+0x15/0x17
[ 2379.263907]  [<ffffffff81162b7a>] mempool_alloc+0x72/0x154
[ 2379.269403]  [<ffffffff810c4b45>] ? lockdep_init_map+0xc9/0x5a3
[ 2379.275354]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2379.280754]  [<ffffffff8133fdc1>] bio_alloc_bioset+0xe8/0x1d7
[ 2379.286535]  [<ffffffff81643127>] kcryptd_crypt+0x1ab/0x325
[ 2379.292143]  [<ffffffff810998fd>] ? process_one_work+0x1ad/0x4e2
[ 2379.298208]  [<ffffffff810999d3>] process_one_work+0x283/0x4e2
[ 2379.304117]  [<ffffffff810c502d>] ? put_lock_stats.isra.9+0xe/0x20
[ 2379.310341]  [<ffffffff8109a860>] worker_thread+0x285/0x370
[ 2379.315946]  [<ffffffff8109a5db>] ? rescuer_thread+0x2d1/0x2d1
[ 2379.321840]  [<ffffffff8109f208>] kthread+0xff/0x107
[ 2379.326825]  [<ffffffff8184b1f2>] ret_from_fork+0x22/0x50
[ 2379.332299]  [<ffffffff8109f109>] ? kthread_create_on_node+0x1ea/0x1ea

[ 2385.193584] kworker/u8:1    D ffff880022e634b8     0  2342      2 0x00000000
[ 2385.200692] Workqueue: kcryptd kcryptd_crypt
[ 2385.205023]  ffff880022e634b8 00ff88011b3ccd80 ffff88011b3d62d8 ffff88011ae45200
[ 2385.212554]  ffff88011a472900 ffff880022e64000 0000000100098b0a ffff880022e634f0
[ 2385.220052]  ffff88011b3ccd80 ffff8800c5b19350 ffff880022e634d0 ffffffff81845cec
[ 2385.227547] Call Trace:
[ 2385.230002]  [<ffffffff81845cec>] schedule+0x8b/0xa3
[ 2385.235001]  [<ffffffff81849b5b>] schedule_timeout+0x20b/0x285
[ 2385.240893]  [<ffffffff810e6da6>] ? init_timer_key+0x112/0x112
[ 2385.246787]  [<ffffffff81849c33>] schedule_timeout_uninterruptible+0x1e/0x20
[ 2385.253858]  [<ffffffff81849c33>] ? schedule_timeout_uninterruptible+0x1e/0x20
[ 2385.261140]  [<ffffffff8117d72e>] wait_iff_congested+0x92/0x1b4
[ 2385.267083]  [<ffffffff810bdd00>] ? wait_woken+0x72/0x72
[ 2385.272448]  [<ffffffff811745c7>] shrink_inactive_list+0x3dc/0x4a1
[ 2385.278662]  [<ffffffff81174e8b>] shrink_zone_memcg+0x4c1/0x661
[ 2385.284643]  [<ffffffff81175107>] shrink_zone+0xdc/0x1e5
[ 2385.290006]  [<ffffffff81175107>] ? shrink_zone+0xdc/0x1e5
[ 2385.295518]  [<ffffffff811753b5>] do_try_to_free_pages+0x1a5/0x2c3
[ 2385.301739]  [<ffffffff811755f6>] try_to_free_pages+0x123/0x21f
[ 2385.307710]  [<ffffffff81168216>] __alloc_pages_nodemask+0x4c9/0x978
[ 2385.314097]  [<ffffffff8138027a>] ? debug_smp_processor_id+0x17/0x19
[ 2385.320509]  [<ffffffff8119fb2a>] new_slab+0xbc/0x3bb
[ 2385.325598]  [<ffffffff811a1acd>] ___slab_alloc.constprop.22+0x2fb/0x37b
[ 2385.332320]  [<ffffffff8138027a>] ? debug_smp_processor_id+0x17/0x19
[ 2385.338726]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2385.344784]  [<ffffffff8101f5ba>] ? sched_clock+0x9/0xd
[ 2385.350052]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2385.355494]  [<ffffffff810c6438>] ? __lock_acquire.isra.16+0x55e/0xb4c
[ 2385.362063]  [<ffffffff811a1ba4>] __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2385.369247]  [<ffffffff811a1ba4>] ? __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2385.376630]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2385.382689]  [<ffffffff811a1c78>] kmem_cache_alloc+0xa0/0x1d6
[ 2385.388493]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2385.394544]  [<ffffffff81162a88>] mempool_alloc_slab+0x15/0x17
[ 2385.400410]  [<ffffffff81162b7a>] mempool_alloc+0x72/0x154
[ 2385.405915]  [<ffffffff810c4b45>] ? lockdep_init_map+0xc9/0x5a3
[ 2385.411875]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2385.417311]  [<ffffffff8133fdc1>] bio_alloc_bioset+0xe8/0x1d7
[ 2385.423082]  [<ffffffff81643127>] kcryptd_crypt+0x1ab/0x325
[ 2385.428704]  [<ffffffff810998fd>] ? process_one_work+0x1ad/0x4e2
[ 2385.434771]  [<ffffffff810999d3>] process_one_work+0x283/0x4e2
[ 2385.440664]  [<ffffffff810c502d>] ? put_lock_stats.isra.9+0xe/0x20
[ 2385.446904]  [<ffffffff8109a860>] worker_thread+0x285/0x370
[ 2385.452510]  [<ffffffff8109a5db>] ? rescuer_thread+0x2d1/0x2d1
[ 2385.458385]  [<ffffffff8109f208>] kthread+0xff/0x107
[ 2385.463379]  [<ffffffff8184b1f2>] ret_from_fork+0x22/0x50
[ 2385.468776]  [<ffffffff8109f109>] ? kthread_create_on_node+0x1ea/0x1ea

[ 2386.089621] kworker/u8:0    D ffff88010cd434b8     0 15543      2 0x00000000
[ 2386.096770] Workqueue: kcryptd kcryptd_crypt
[ 2386.101060]  ffff88010cd434b8 00ff88011b1ccd80 ffff88011b1d62d8 ffffffff81e1d540
[ 2386.108598]  ffff8800c00ca900 ffff88010cd44000 00000001000982fe ffff88010cd434f0
[ 2386.116102]  ffff88011b1ccd80 ffff8800c5b19350 ffff88010cd434d0 ffffffff81845cec
[ 2386.123651] Call Trace:
[ 2386.126132]  [<ffffffff81845cec>] schedule+0x8b/0xa3
[ 2386.131167]  [<ffffffff81849b5b>] schedule_timeout+0x20b/0x285
[ 2386.137017]  [<ffffffff810e6da6>] ? init_timer_key+0x112/0x112
[ 2386.142902]  [<ffffffff81849c33>] schedule_timeout_uninterruptible+0x1e/0x20
[ 2386.149999]  [<ffffffff81849c33>] ? schedule_timeout_uninterruptible+0x1e/0x20
[ 2386.157271]  [<ffffffff8117d72e>] wait_iff_congested+0x92/0x1b4
[ 2386.163258]  [<ffffffff810bdd00>] ? wait_woken+0x72/0x72
[ 2386.168622]  [<ffffffff811745c7>] shrink_inactive_list+0x3dc/0x4a1
[ 2386.174862]  [<ffffffff81174e8b>] shrink_zone_memcg+0x4c1/0x661
[ 2386.180834]  [<ffffffff81175107>] shrink_zone+0xdc/0x1e5
[ 2386.186154]  [<ffffffff81175107>] ? shrink_zone+0xdc/0x1e5
[ 2386.191691]  [<ffffffff811753b5>] do_try_to_free_pages+0x1a5/0x2c3
[ 2386.197931]  [<ffffffff811755f6>] try_to_free_pages+0x123/0x21f
[ 2386.203893]  [<ffffffff81168216>] __alloc_pages_nodemask+0x4c9/0x978
[ 2386.210314]  [<ffffffff811a1776>] ? get_partial_node.isra.19+0x353/0x3af
[ 2386.217057]  [<ffffffff8119fb97>] new_slab+0x129/0x3bb
[ 2386.222246]  [<ffffffff811a1acd>] ___slab_alloc.constprop.22+0x2fb/0x37b
[ 2386.228979]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2386.235039]  [<ffffffff81049da2>] ? glue_xts_crypt_128bit+0x1a6/0x1d8
[ 2386.241529]  [<ffffffff8101f5ba>] ? sched_clock+0x9/0xd
[ 2386.246790]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2386.252248]  [<ffffffff810c6438>] ? __lock_acquire.isra.16+0x55e/0xb4c
[ 2386.258863]  [<ffffffff811a1ba4>] __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2386.266027]  [<ffffffff811a1ba4>] ? __slab_alloc.isra.17.constprop.21+0x57/0x8b
[ 2386.273358]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2386.279383]  [<ffffffff811a1c78>] kmem_cache_alloc+0xa0/0x1d6
[ 2386.285196]  [<ffffffff81162a88>] ? mempool_alloc_slab+0x15/0x17
[ 2386.291246]  [<ffffffff81162a88>] mempool_alloc_slab+0x15/0x17
[ 2386.297146]  [<ffffffff81162b7a>] mempool_alloc+0x72/0x154
[ 2386.302669]  [<ffffffff810c4b45>] ? lockdep_init_map+0xc9/0x5a3
[ 2386.308622]  [<ffffffff810ae420>] ? local_clock+0x20/0x22
[ 2386.314073]  [<ffffffff8133fdc1>] bio_alloc_bioset+0xe8/0x1d7
[ 2386.319843]  [<ffffffff81643127>] kcryptd_crypt+0x1ab/0x325
[ 2386.325443]  [<ffffffff810998fd>] ? process_one_work+0x1ad/0x4e2
[ 2386.331482]  [<ffffffff810999d3>] process_one_work+0x283/0x4e2
[ 2386.337331]  [<ffffffff810c502d>] ? put_lock_stats.isra.9+0xe/0x20
[ 2386.343528]  [<ffffffff8109a860>] worker_thread+0x285/0x370
[ 2386.349143]  [<ffffffff8109a5db>] ? rescuer_thread+0x2d1/0x2d1
[ 2386.355055]  [<ffffffff8109f208>] kthread+0xff/0x107
[ 2386.360048]  [<ffffffff8184b1f2>] ret_from_fork+0x22/0x50
[ 2386.365471]  [<ffffffff8109f109>] ? kthread_create_on_node+0x1ea/0x1ea

[ 2419.047134] workqueue kcryptd: flags=0x2a
[ 2419.051178]   pwq 8: cpus=0-3 flags=0x4 nice=0 active=4/4
[ 2419.056687]     in-flight: 1592:kcryptd_crypt, 1594:kcryptd_crypt, 2342:kcryptd_crypt, 15543:kcryptd_crypt
[ 2419.066479]     delayed: kcryptd_crypt, kcryptd_crypt, (...snipped...) kcryptd_crypt, (...too long to finish...)

Why can't we stop queuing so many kcryptd_crypt work requests?

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

#1442455

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2016-07-13 15:30 +0200
Message-ID<rUsoG-3Co-23@gated-at.bofh.it>
In reply to#1441921
> On Tue, 12 Jul 2016, Michal Hocko wrote:
> 
>> On Mon 11-07-16 11:43:02, Mikulas Patocka wrote:
>> [...]
>>> The general problem is that the memory allocator does 16 retries to 
>>> allocate a page and then triggers the OOM killer (and it doesn't take into 
>>> account how much swap space is free or how many dirty pages were really 
>>> swapped out while it waited).
>>
>> Well, that is not how it works exactly. We retry as long as there is a
>> reclaim progress (at least one page freed) back off only if the
>> reclaimable memory can exceed watermks which is scaled down in 16
>> retries. The overal size of free swap is not really that important if we
>> cannot swap out like here due to complete memory reserves depletion:
>> https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/vmlog-1462458369-00000/sample-00011/dmesg:
>> [   90.491276] Node 0 DMA free:0kB min:60kB low:72kB high:84kB active_anon:4096kB inactive_anon:4636kB active_file:212kB inactive_file:280kB unevictable:488kB isolated(anon):0kB isolated(file):0kB present:15992kB managed:15908kB mlocked:488kB dirty:276kB writeback:4636kB mapped:476kB shmem:12kB slab_reclaimable:204kB slab_unreclaimable:4700kB kernel_stack:48kB pagetables:120kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:61132 all_unreclaimable? yes
>> [   90.491283] lowmem_reserve[]: 0 977 977 977
>> [   90.491286] Node 0 DMA32 free:0kB min:3828kB low:4824kB high:5820kB active_anon:423820kB inactive_anon:424916kB active_file:17996kB inactive_file:21800kB unevictable:20724kB isolated(anon):384kB isolated(file):0kB present:1032184kB managed:1001260kB mlocked:20724kB dirty:25236kB writeback:49972kB mapped:23076kB shmem:1364kB slab_reclaimable:13796kB slab_unreclaimable:43008kB kernel_stack:2816kB pagetables:7320kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:5635400 all_unreclaimable? yes
>>
>> Look at the amount of free memory. It is completely depleted. So it
>> smells like a process which has access to memory reserves has consumed
>> all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq
>> context user which went off the leash.
> 
> It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. Prior 
> to this commit, mempool allocations set __GFP_NOMEMALLOC, so they never 
> exhausted reserved memory. With this commit, mempool allocations drop 
> __GFP_NOMEMALLOC, so they can dig deeper (if the process has PF_MEMALLOC, 
> they can bypass all limits).

I wonder whether commit f9054c70d28bc214 ("mm, mempool: only set
__GFP_NOMEMALLOC if there are free elements") is doing correct thing.
It says

    If an oom killed thread calls mempool_alloc(), it is possible that it'll
    loop forever if there are no elements on the freelist since
    __GFP_NOMEMALLOC prevents it from accessing needed memory reserves in
    oom conditions.

but we can allow mempool_alloc(__GFP_NOMEMALLOC) requests to access
memory reserves via below change, can't we? The purpose of allowing
ALLOC_NO_WATERMARKS via TIF_MEMDIE is to make sure current allocation
request does not to loop forever inside the page allocator, isn't it?
Why we need to allow mempool_alloc(__GFP_NOMEMALLOC) requests to use
ALLOC_NO_WATERMARKS when TIF_MEMDIE is not set?

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6903b69..e4e3700 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3439,14 +3439,14 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
 	} else if (unlikely(rt_task(current)) && !in_interrupt())
 		alloc_flags |= ALLOC_HARDER;
 
-	if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
+	if (!in_interrupt() && unlikely(test_thread_flag(TIF_MEMDIE)))
+		alloc_flags |= ALLOC_NO_WATERMARKS;
+	else if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
 		if (gfp_mask & __GFP_MEMALLOC)
 			alloc_flags |= ALLOC_NO_WATERMARKS;
 		else if (in_serving_softirq() && (current->flags & PF_MEMALLOC))
 			alloc_flags |= ALLOC_NO_WATERMARKS;
-		else if (!in_interrupt() &&
-				((current->flags & PF_MEMALLOC) ||
-				 unlikely(test_thread_flag(TIF_MEMDIE))))
+		else if (!in_interrupt() && (current->flags & PF_MEMALLOC))
 			alloc_flags |= ALLOC_NO_WATERMARKS;
 	}
 #ifdef CONFIG_CMA

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

#1442465

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-13 15:50 +0200
Message-ID<rUsI1-3Kr-5@gated-at.bofh.it>
In reply to#1442455
[CC David]

On Wed 13-07-16 22:19:23, Tetsuo Handa wrote:
> >> On Mon 11-07-16 11:43:02, Mikulas Patocka wrote:
> >> [...]
> >>> The general problem is that the memory allocator does 16 retries to 
> >>> allocate a page and then triggers the OOM killer (and it doesn't take into 
> >>> account how much swap space is free or how many dirty pages were really 
> >>> swapped out while it waited).
> >>
> >> Well, that is not how it works exactly. We retry as long as there is a
> >> reclaim progress (at least one page freed) back off only if the
> >> reclaimable memory can exceed watermks which is scaled down in 16
> >> retries. The overal size of free swap is not really that important if we
> >> cannot swap out like here due to complete memory reserves depletion:
> >> https://okozina.fedorapeople.org/bugs/swap_on_dmcrypt/vmlog-1462458369-00000/sample-00011/dmesg:
> >> [   90.491276] Node 0 DMA free:0kB min:60kB low:72kB high:84kB active_anon:4096kB inactive_anon:4636kB active_file:212kB inactive_file:280kB unevictable:488kB isolated(anon):0kB isolated(file):0kB present:15992kB managed:15908kB mlocked:488kB dirty:276kB writeback:4636kB mapped:476kB shmem:12kB slab_reclaimable:204kB slab_unreclaimable:4700kB kernel_stack:48kB pagetables:120kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:61132 all_unreclaimable? yes
> >> [   90.491283] lowmem_reserve[]: 0 977 977 977
> >> [   90.491286] Node 0 DMA32 free:0kB min:3828kB low:4824kB high:5820kB active_anon:423820kB inactive_anon:424916kB active_file:17996kB inactive_file:21800kB unevictable:20724kB isolated(anon):384kB isolated(file):0kB present:1032184kB managed:1001260kB mlocked:20724kB dirty:25236kB writeback:49972kB mapped:23076kB shmem:1364kB slab_reclaimable:13796kB slab_unreclaimable:43008kB kernel_stack:2816kB pagetables:7320kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:5635400 all_unreclaimable? yes
> >>
> >> Look at the amount of free memory. It is completely depleted. So it
> >> smells like a process which has access to memory reserves has consumed
> >> all of it. I suspect a __GFP_MEMALLOC resp. PF_MEMALLOC from softirq
> >> context user which went off the leash.
> > 
> > It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. Prior 
> > to this commit, mempool allocations set __GFP_NOMEMALLOC, so they never 
> > exhausted reserved memory. With this commit, mempool allocations drop 
> > __GFP_NOMEMALLOC, so they can dig deeper (if the process has PF_MEMALLOC, 
> > they can bypass all limits).
> 
> I wonder whether commit f9054c70d28bc214 ("mm, mempool: only set
> __GFP_NOMEMALLOC if there are free elements") is doing correct thing.
> It says
> 
>     If an oom killed thread calls mempool_alloc(), it is possible that it'll
>     loop forever if there are no elements on the freelist since
>     __GFP_NOMEMALLOC prevents it from accessing needed memory reserves in
>     oom conditions.

I haven't studied the patch very deeply so I might be missing something
but from a quick look the patch does exactly what the above says.

mempool_alloc used to inhibit ALLOC_NO_WATERMARKS by default. David has
only changed that to allow ALLOC_NO_WATERMARKS if there are no objects
in the pool and so we have no fallback for the default __GFP_NORETRY
request.

> but we can allow mempool_alloc(__GFP_NOMEMALLOC) requests to access
> memory reserves via below change, can't we?

Well, I do not see all the potential side effects of such a change but
I believe it shouldn't be really necessary because we should eventually
allow ALLOC_NO_WATERMARKS even from mempool_alloc.

> The purpose of allowing
> ALLOC_NO_WATERMARKS via TIF_MEMDIE is to make sure current allocation
> request does not to loop forever inside the page allocator, isn't it?
> Why we need to allow mempool_alloc(__GFP_NOMEMALLOC) requests to use
> ALLOC_NO_WATERMARKS when TIF_MEMDIE is not set?
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6903b69..e4e3700 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3439,14 +3439,14 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
>  	} else if (unlikely(rt_task(current)) && !in_interrupt())
>  		alloc_flags |= ALLOC_HARDER;
>  
> -	if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
> +	if (!in_interrupt() && unlikely(test_thread_flag(TIF_MEMDIE)))
> +		alloc_flags |= ALLOC_NO_WATERMARKS;
> +	else if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
>  		if (gfp_mask & __GFP_MEMALLOC)
>  			alloc_flags |= ALLOC_NO_WATERMARKS;
>  		else if (in_serving_softirq() && (current->flags & PF_MEMALLOC))
>  			alloc_flags |= ALLOC_NO_WATERMARKS;
> -		else if (!in_interrupt() &&
> -				((current->flags & PF_MEMALLOC) ||
> -				 unlikely(test_thread_flag(TIF_MEMDIE))))
> +		else if (!in_interrupt() && (current->flags & PF_MEMALLOC))
>  			alloc_flags |= ALLOC_NO_WATERMARKS;
>  	}
>  #ifdef CONFIG_CMA
> 
> --
> 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>

-- 
Michal Hocko
SUSE Labs

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

#1442500

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-07-13 16:30 +0200
Message-ID<rUtkK-4fl-25@gated-at.bofh.it>
In reply to#1442465

On Wed, 13 Jul 2016, Michal Hocko wrote:

> [CC David]
> 
> > > It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. 
> > > Prior to this commit, mempool allocations set __GFP_NOMEMALLOC, so 
> > > they never exhausted reserved memory. With this commit, mempool 
> > > allocations drop __GFP_NOMEMALLOC, so they can dig deeper (if the 
> > > process has PF_MEMALLOC, they can bypass all limits).
> > 
> > I wonder whether commit f9054c70d28bc214 ("mm, mempool: only set 
> > __GFP_NOMEMALLOC if there are free elements") is doing correct thing. 
> > It says
> > 
> >     If an oom killed thread calls mempool_alloc(), it is possible that 
> > it'll
> >     loop forever if there are no elements on the freelist since
> >     __GFP_NOMEMALLOC prevents it from accessing needed memory reserves in
> >     oom conditions.
> 
> I haven't studied the patch very deeply so I might be missing something
> but from a quick look the patch does exactly what the above says.
> 
> mempool_alloc used to inhibit ALLOC_NO_WATERMARKS by default. David has
> only changed that to allow ALLOC_NO_WATERMARKS if there are no objects
> in the pool and so we have no fallback for the default __GFP_NORETRY
> request.

The swapper core sets the flag PF_MEMALLOC and calls generic_make_request 
to submit the swapping bio to the block driver. The device mapper driver 
uses mempools for all its I/O processing.

Prior to the patch f9054c70d28bc214b2857cf8db8269f4f45a5e23, mempool_alloc 
never exhausted the reserved memory - it tried to allocace first with 
__GFP_NOMEMALLOC (thus preventing the allocator from allocating below the 
limits), then it tried to allocate from the mempool reserve and if the 
mempool is exhausted, it waits until some structures are returned to the 
mempool.

After the patch f9054c70d28bc214b2857cf8db8269f4f45a5e23, __GFP_NOMEMALLOC 
is not used if the mempool is exhausted - and so repeated use of 
mempool_alloc (tohether with PF_MEMALLOC that is implicitly set) can 
exhaust all available memory.

The patch f9054c70d28bc214b2857cf8db8269f4f45a5e23 allows more paralellism 
(mempool_alloc waits less and proceeds more often), but the downside is 
that it exhausts all the memory. Bisection showed that those dm-crypt 
swapping failures were caused by that patch.

I think f9054c70d28bc214b2857cf8db8269f4f45a5e23 should be reverted - but 
first, we need to find out why does swapping fail if all the memory is 
exhausted - that is a separate bug that should be addressed first.

> > but we can allow mempool_alloc(__GFP_NOMEMALLOC) requests to access
> > memory reserves via below change, can't we?

There are no mempool_alloc(__GFP_NOMEMALLOC) requsts - mempool users don't 
use this flag.

Mikulas

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

#1442518

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-13 17:00 +0200
Message-ID<rUtNL-4qs-1@gated-at.bofh.it>
In reply to#1442500
On Wed 13-07-16 10:18:35, Mikulas Patocka wrote:
> 
> 
> On Wed, 13 Jul 2016, Michal Hocko wrote:
> 
> > [CC David]
> > 
> > > > It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. 
> > > > Prior to this commit, mempool allocations set __GFP_NOMEMALLOC, so 
> > > > they never exhausted reserved memory. With this commit, mempool 
> > > > allocations drop __GFP_NOMEMALLOC, so they can dig deeper (if the 
> > > > process has PF_MEMALLOC, they can bypass all limits).
> > > 
> > > I wonder whether commit f9054c70d28bc214 ("mm, mempool: only set 
> > > __GFP_NOMEMALLOC if there are free elements") is doing correct thing. 
> > > It says
> > > 
> > >     If an oom killed thread calls mempool_alloc(), it is possible that 
> > > it'll
> > >     loop forever if there are no elements on the freelist since
> > >     __GFP_NOMEMALLOC prevents it from accessing needed memory reserves in
> > >     oom conditions.
> > 
> > I haven't studied the patch very deeply so I might be missing something
> > but from a quick look the patch does exactly what the above says.
> > 
> > mempool_alloc used to inhibit ALLOC_NO_WATERMARKS by default. David has
> > only changed that to allow ALLOC_NO_WATERMARKS if there are no objects
> > in the pool and so we have no fallback for the default __GFP_NORETRY
> > request.
> 
> The swapper core sets the flag PF_MEMALLOC and calls generic_make_request 
> to submit the swapping bio to the block driver. The device mapper driver 
> uses mempools for all its I/O processing.

OK, this is the part I have missed. I didn't realize that the swapout
path, which is indeed PF_MEMALLOC, can get down to blk code which uses
mempools. A quick code travers shows that at least
	make_request_fn = blk_queue_bio
	blk_queue_bio
	  get_request
	    __get_request

might do that. And in that case I agree that the above mentioned patch
has unintentional side effects and should be re-evaluated. David, what
do you think? An obvious fixup would be considering TIF_MEMDIE in
mempool_alloc explicitly.

-- 
Michal Hocko
SUSE Labs

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

#1442539

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-07-13 17:20 +0200
Message-ID<rUu78-4Nq-7@gated-at.bofh.it>
In reply to#1442518

On Wed, 13 Jul 2016, Michal Hocko wrote:

> On Wed 13-07-16 10:18:35, Mikulas Patocka wrote:
> > 
> > 
> > On Wed, 13 Jul 2016, Michal Hocko wrote:
> > 
> > > [CC David]
> > > 
> > > > > It is caused by the commit f9054c70d28bc214b2857cf8db8269f4f45a5e23. 
> > > > > Prior to this commit, mempool allocations set __GFP_NOMEMALLOC, so 
> > > > > they never exhausted reserved memory. With this commit, mempool 
> > > > > allocations drop __GFP_NOMEMALLOC, so they can dig deeper (if the 
> > > > > process has PF_MEMALLOC, they can bypass all limits).
> > > > 
> > > > I wonder whether commit f9054c70d28bc214 ("mm, mempool: only set 
> > > > __GFP_NOMEMALLOC if there are free elements") is doing correct thing. 
> > > > It says
> > > > 
> > > >     If an oom killed thread calls mempool_alloc(), it is possible that 
> > > > it'll
> > > >     loop forever if there are no elements on the freelist since
> > > >     __GFP_NOMEMALLOC prevents it from accessing needed memory reserves in
> > > >     oom conditions.
> > > 
> > > I haven't studied the patch very deeply so I might be missing something
> > > but from a quick look the patch does exactly what the above says.
> > > 
> > > mempool_alloc used to inhibit ALLOC_NO_WATERMARKS by default. David has
> > > only changed that to allow ALLOC_NO_WATERMARKS if there are no objects
> > > in the pool and so we have no fallback for the default __GFP_NORETRY
> > > request.
> > 
> > The swapper core sets the flag PF_MEMALLOC and calls generic_make_request 
> > to submit the swapping bio to the block driver. The device mapper driver 
> > uses mempools for all its I/O processing.
> 
> OK, this is the part I have missed. I didn't realize that the swapout
> path, which is indeed PF_MEMALLOC, can get down to blk code which uses
> mempools. A quick code travers shows that at least
> 	make_request_fn = blk_queue_bio
> 	blk_queue_bio
> 	  get_request
> 	    __get_request
> 
> might do that. And in that case I agree that the above mentioned patch
> has unintentional side effects and should be re-evaluated. David, what
> do you think? An obvious fixup would be considering TIF_MEMDIE in
> mempool_alloc explicitly.

What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
tries to fix?

Do you have a stacktrace where it deadlocked, or was just a theoretical 
consideration?

Mempool users generally (except for some flawed cases like fs_bio_set) do 
not require memory to proceed. So if you just loop in mempool_alloc, the 
processes that exhasted the mempool reserve will eventually return objects 
to the mempool and you should proceed.

If you can't proceed, it is a bug in the code that uses the mempool.

Mikulas

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

#1442947

FromDavid Rientjes <rientjes@google.com>
Date2016-07-14 02:00 +0200
Message-ID<rUCel-1CC-7@gated-at.bofh.it>
In reply to#1442539
On Wed, 13 Jul 2016, Mikulas Patocka wrote:

> What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> tries to fix?
> 

It prevents the whole system from livelocking due to an oom killed process 
stalling forever waiting for mempool_alloc() to return.  No other threads 
may be oom killed while waiting for it to exit.

> Do you have a stacktrace where it deadlocked, or was just a theoretical 
> consideration?
> 

schedule
schedule_timeout
io_schedule_timeout
mempool_alloc
__split_and_process_bio
dm_request
generic_make_request
submit_bio
mpage_readpages
ext4_readpages
__do_page_cache_readahead
ra_submit
filemap_fault
handle_mm_fault
__do_page_fault
do_page_fault
page_fault

> Mempool users generally (except for some flawed cases like fs_bio_set) do 
> not require memory to proceed. So if you just loop in mempool_alloc, the 
> processes that exhasted the mempool reserve will eventually return objects 
> to the mempool and you should proceed.
> 

That's obviously not the case if we have hundreds of machines timing out 
after two hours waiting for that fault to succeed.  The mempool interface 
cannot require that users return elements to the pool synchronous with all 
allocators so that we can happily loop forever, the only requirement on 
the interface is that mempool_alloc() must succeed.  If the context of the 
thread doing mempool_alloc() allows access to memory reserves, this will 
always be allowed by the page allocator.  This is not a mempool problem.

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

#1443358

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2016-07-14 13:10 +0200
Message-ID<rUMGK-ue-15@gated-at.bofh.it>
In reply to#1442947
Michal Hocko wrote:
> OK, this is the part I have missed. I didn't realize that the swapout
> path, which is indeed PF_MEMALLOC, can get down to blk code which uses
> mempools. A quick code travers shows that at least
> 	make_request_fn = blk_queue_bio
> 	blk_queue_bio
> 	  get_request
> 	    __get_request
> 
> might do that. And in that case I agree that the above mentioned patch
> has unintentional side effects and should be re-evaluated. David, what
> do you think? An obvious fixup would be considering TIF_MEMDIE in
> mempool_alloc explicitly.

TIF_MEMDIE is racy. Since the OOM killer sets TIF_MEMDIE on only one thread,
there is no guarantee that TIF_MEMDIE is set to the thread which is looping
inside mempool_alloc(). And since __GFP_NORETRY is used (regardless of
f9054c70d28bc214), out_of_memory() is not called via __alloc_pages_may_oom().
This means that the thread which is looping inside mempool_alloc() can't
get TIF_MEMDIE unless TIF_MEMDIE is set by the OOM killer.

Maybe set __GFP_NOMEMALLOC by default at mempool_alloc() and remove it
at mempool_alloc() when fatal_signal_pending() is true? But that behavior
can OOM-kill somebody else when current was not OOM-killed. Sigh...

David Rientjes wrote:
> On Wed, 13 Jul 2016, Mikulas Patocka wrote:
> 
> > What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> > tries to fix?
> > 
> 
> It prevents the whole system from livelocking due to an oom killed process 
> stalling forever waiting for mempool_alloc() to return.  No other threads 
> may be oom killed while waiting for it to exit.

Is that concern still valid? We have the OOM reaper for CONFIG_MMU=y case.

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

#1443407

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-07-14 14:30 +0200
Message-ID<rUNWa-1dc-7@gated-at.bofh.it>
In reply to#1443358

On Thu, 14 Jul 2016, Tetsuo Handa wrote:

> Michal Hocko wrote:
> > OK, this is the part I have missed. I didn't realize that the swapout
> > path, which is indeed PF_MEMALLOC, can get down to blk code which uses
> > mempools. A quick code travers shows that at least
> > 	make_request_fn = blk_queue_bio
> > 	blk_queue_bio
> > 	  get_request
> > 	    __get_request
> > 
> > might do that. And in that case I agree that the above mentioned patch
> > has unintentional side effects and should be re-evaluated. David, what
> > do you think? An obvious fixup would be considering TIF_MEMDIE in
> > mempool_alloc explicitly.
> 
> TIF_MEMDIE is racy. Since the OOM killer sets TIF_MEMDIE on only one thread,
> there is no guarantee that TIF_MEMDIE is set to the thread which is looping
> inside mempool_alloc().

If the device mapper subsystem is not returning objects to the mempool, it 
should be investigated as a bug in the device mapper.

There is no need to add workarounds to mempool_alloc to work around that 
bug.

Mikulas

> And since __GFP_NORETRY is used (regardless of
> f9054c70d28bc214), out_of_memory() is not called via __alloc_pages_may_oom().
> This means that the thread which is looping inside mempool_alloc() can't
> get TIF_MEMDIE unless TIF_MEMDIE is set by the OOM killer.
> 
> Maybe set __GFP_NOMEMALLOC by default at mempool_alloc() and remove it
> at mempool_alloc() when fatal_signal_pending() is true? But that behavior
> can OOM-kill somebody else when current was not OOM-killed. Sigh...
> 
> David Rientjes wrote:
> > On Wed, 13 Jul 2016, Mikulas Patocka wrote:
> > 
> > > What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> > > tries to fix?
> > > 
> > 
> > It prevents the whole system from livelocking due to an oom killed process 
> > stalling forever waiting for mempool_alloc() to return.  No other threads 
> > may be oom killed while waiting for it to exit.
> 
> Is that concern still valid? We have the OOM reaper for CONFIG_MMU=y case.
>

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

#1443753

FromDavid Rientjes <rientjes@google.com>
Date2016-07-14 22:30 +0200
Message-ID<rUVqH-5SB-51@gated-at.bofh.it>
In reply to#1443358
On Thu, 14 Jul 2016, Tetsuo Handa wrote:

> David Rientjes wrote:
> > On Wed, 13 Jul 2016, Mikulas Patocka wrote:
> > 
> > > What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> > > tries to fix?
> > > 
> > 
> > It prevents the whole system from livelocking due to an oom killed process 
> > stalling forever waiting for mempool_alloc() to return.  No other threads 
> > may be oom killed while waiting for it to exit.
> 
> Is that concern still valid? We have the OOM reaper for CONFIG_MMU=y case.
> 

Umm, show me an explicit guarantee where the oom reaper will free memory 
such that other threads may return memory to this process's mempool so it 
can make forward progress in mempool_alloc() without the need of utilizing 
memory reserves.  First, it might be helpful to show that the oom reaper 
is ever guaranteed to free any memory for a selected oom victim.

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

#1443799

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2016-07-14 23:50 +0200
Message-ID<rUWG5-6zn-17@gated-at.bofh.it>
In reply to#1443753
David Rientjes wrote:
> On Thu, 14 Jul 2016, Tetsuo Handa wrote:
> 
> > David Rientjes wrote:
> > > On Wed, 13 Jul 2016, Mikulas Patocka wrote:
> > > 
> > > > What are the real problems that f9054c70d28bc214b2857cf8db8269f4f45a5e23 
> > > > tries to fix?
> > > > 
> > > 
> > > It prevents the whole system from livelocking due to an oom killed process 
> > > stalling forever waiting for mempool_alloc() to return.  No other threads 
> > > may be oom killed while waiting for it to exit.
> > 
> > Is that concern still valid? We have the OOM reaper for CONFIG_MMU=y case.
> > 
> 
> Umm, show me an explicit guarantee where the oom reaper will free memory 
> such that other threads may return memory to this process's mempool so it 
> can make forward progress in mempool_alloc() without the need of utilizing 
> memory reserves.  First, it might be helpful to show that the oom reaper 
> is ever guaranteed to free any memory for a selected oom victim.
> 

Whether the OOM reaper will free some memory no longer matters. Instead,
whether the OOM reaper will let the OOM killer select next OOM victim matters.

Are you aware that the OOM reaper will let the OOM killer select next OOM
victim (currently by clearing TIF_MEMDIE)? Clearing TIF_MEMDIE in 4.6 occurred
only when OOM reaping succeeded. But we are going to change the OOM reaper
always clear TIF_MEMDIE in 4.8 (or presumably change the OOM killer not to
depend on TIF_MEMDIE) so that the OOM reaper guarantees that the OOM killer
always selects next OOM victim.

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

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web