linux-kernel - Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <alpine.LRH.2.02.1607270948040.1779@file01.intranet.prod.int.rdu2.redhat.com>
Date:	Wed, 27 Jul 2016 10:28:40 -0400 (EDT)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	NeilBrown <neilb@...e.com>
cc:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	LKML <linux-kernel@...r.kernel.org>,
	Michal Hocko <mhocko@...nel.org>, linux-mm@...ck.org,
	dm-devel@...hat.com, Mel Gorman <mgorman@...e.de>,
	David Rientjes <rientjes@...gle.com>,
	Ondrej Kozina <okozina@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle
 PF_LESS_THROTTLE tasks

On Wed, 27 Jul 2016, NeilBrown wrote:

> On Tue, Jul 26 2016, Mikulas Patocka wrote:
> 
> > On Sat, 23 Jul 2016, NeilBrown wrote:
> >
> >> "dirtying ... from the reclaim context" ??? What does that mean?
> >> According to
> >>   Commit: 26eecbf3543b ("[PATCH] vm: pageout throttling")
> >> From the history tree, the purpose of throttle_vm_writeout() is to
> >> limit the amount of memory that is concurrently under I/O.
> >> That seems strange to me because I thought it was the responsibility of
> >> each backing device to impose a limit - a maximum queue size of some
> >> sort.
> >
> > Device mapper doesn't impose any limit for in-flight bios.
> 
> I would suggest that it probably should. At least it should
> "set_wb_congested()" when the number of in-flight bios reaches some
> arbitrary threshold.

If we set the device mapper device as congested, it can again trigger that 
mempool alloc throttling bug.

I.e. suppose that we swap to a dm-crypt device. The dm-crypt device 
becomes clogged and sets its state as congested. The underlying block 
device is not congested.

The mempool_alloc function in the dm-crypt workqueue sets the 
PF_LESS_THROTTLE flag, and tries to allocate memory, but according to 
Michal's patches, processes with PF_LESS_THROTTLE may still get throttled.

So if we set the dm-crypt device as congested, it can incorrectly throttle 
the dm-crypt workqueue that does allocations of temporary pages and 
encryption.

I think that approach with PF_LESS_THROTTLE in mempool_alloc is incorrect 
and that mempool allocations should never be throttled.

> > I've made some patches that limit in-flight bios for device mapper in
> > the past, but there were not integrated into upstream.
> 
> I second the motion to resurrect these.

I uploaded those patches here:

http://people.redhat.com/~mpatocka/patches/kernel/dm-limit-outstanding-bios/

Mikulas

> Thanks,
> NeilBrown
>