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


Groups > linux.kernel > #1450036 > unrolled thread

Re: [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

Started byMikulas Patocka <mpatocka@redhat.com>
First post2016-07-26 00:00 +0200
Last post2016-07-27 23:40 +0200
Articles 9 — 3 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: [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE  tasks Mikulas Patocka <mpatocka@redhat.com> - 2016-07-26 00:00 +0200
    Re: [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE  tasks Michal Hocko <mhocko@kernel.org> - 2016-07-26 09:30 +0200
    Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks NeilBrown <neilb@suse.com> - 2016-07-27 06:10 +0200
      Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle  PF_LESS_THROTTLE tasks Mikulas Patocka <mpatocka@redhat.com> - 2016-07-27 16:30 +0200
        Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle  PF_LESS_THROTTLE tasks Michal Hocko <mhocko@kernel.org> - 2016-07-27 20:50 +0200
          Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle  PF_LESS_THROTTLE tasks Mikulas Patocka <mpatocka@redhat.com> - 2016-08-03 16:30 +0200
            Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle  PF_LESS_THROTTLE tasks Michal Hocko <mhocko@kernel.org> - 2016-08-03 16:50 +0200
              Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle  PF_LESS_THROTTLE tasks Mikulas Patocka <mpatocka@redhat.com> - 2016-08-04 20:50 +0200
        Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks NeilBrown <neilb@suse.com> - 2016-07-27 23:40 +0200

#1450036 — Re: [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-07-26 00:00 +0200
SubjectRe: [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<rYW4N-1xV-21@gated-at.bofh.it>

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.

Some simple device mapper targets (such as linear or stripe) pass bio 
directly to the underlying device with generic_make_request, so if the 
underlying device's request limit is reached, the target's request routine 
waits.

However, complex dm targets (such as dm-crypt, dm-mirror, dm-thin) pass 
bios to a workqueue that processes them. And since there is no limit on 
the number of workqueue entries, there is no limit on the number of 
in-flight bios.

I've seen a case when I had a HPFS filesystem on dm-crypt. I wrote to the 
filesystem, there was about 2GB dirty data. The HPFS filesystem used 
512-byte bios. dm-crypt allocates one temporary page for each incoming 
bio. So, there were 4M bios in flight, each bio allocated 4k temporary 
page - that is attempted 16GB allocation. It didn't trigger OOM condition 
(because mempool allocations don't ever trigger it), but it temporarily 
exhausted all computer's memory.

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

> If a thread is only making transient allocations, ones which will be
> freed shortly afterwards (not, for example, put in a cache), then I
> don't think it needs to be throttled at all.  I think this universally
> applies to mempools.
> In the case of dm_crypt, if it is writing too fast it will eventually be
> throttled in generic_make_request when the underlying device has a full
> queue and so blocks waiting for requests to be completed, and thus parts
> of them returned to the mempool.

No, it won't be throttled.

dm-crypt does:
1. pass the bio to the encryption workqueue
2. allocate the outgoing bio and allocate temporary pages for the 
   encrypted data
3. do the encryption
4. pass the bio to the writer thread
5. submit the write request with generic_make_request

So, if the underlying block device is throttled, it stalls the writer 
thread, but it doesn't stall the encryption threads and it doesn't stall 
the caller that submits the bios to dm-crypt.

There can be really high number of in-flight bios for dm-crypt.

Mikulas

[toc] | [next] | [standalone]


#1450436

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-26 09:30 +0200
Message-ID<rZ4Yp-7w9-7@gated-at.bofh.it>
In reply to#1450036
On Mon 25-07-16 17:52:17, 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.
> 
> Some simple device mapper targets (such as linear or stripe) pass bio 
> directly to the underlying device with generic_make_request, so if the 
> underlying device's request limit is reached, the target's request routine 
> waits.
> 
> However, complex dm targets (such as dm-crypt, dm-mirror, dm-thin) pass 
> bios to a workqueue that processes them. And since there is no limit on 
> the number of workqueue entries, there is no limit on the number of 
> in-flight bios.
> 
> I've seen a case when I had a HPFS filesystem on dm-crypt. I wrote to the 
> filesystem, there was about 2GB dirty data. The HPFS filesystem used 
> 512-byte bios. dm-crypt allocates one temporary page for each incoming 
> bio. So, there were 4M bios in flight, each bio allocated 4k temporary 
> page - that is attempted 16GB allocation. It didn't trigger OOM condition 
> (because mempool allocations don't ever trigger it), but it temporarily 
> exhausted all computer's memory.

OK, that is certainly not good and something that throttle_vm_writeout
aimed at protecting from. It is a little bit poor protection because
it might fire much more earlier than necessary. Shouldn't those workers
simply backoff when the underlying bdi is congested? It wouldn't help
to queue more IO when the bdi is hammered already.
 
> I've made some patches that limit in-flight bios for device mapper in the 
> past, but there were not integrated into upstream.

Care to revive them? I am not an expert in dm but unbounded amount of
inflight IO doesn't really sound good.

[...]
-- 
Michal Hocko
SUSE Labs

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


#1451030 — Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromNeilBrown <neilb@suse.com>
Date2016-07-27 06:10 +0200
SubjectRe: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<rZokp-2TG-1@gated-at.bofh.it>
In reply to#1450036

[Multipart message — attachments visible in raw view] — view raw

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.

The write-back throttling needs this to get an estimate of how fast the
backing device is, so it can share the dirty_threshold space fairly
among the different backing devices.

I added an arbitrary limit to raid1 back in 2011 (34db0cd60f8a1f)
because the lack of a limit was causing problems.
Specifically the write queue would get so long that ext3 would block for
an extended period when trying to flush a transaction, and that blocked
lots of other things, like atime updates.

Maybe there have been other fixes since then to other parts of the
puzzle, but the congestion tracking still seems to be an important part
of the picture and I think it would be best if every bdi would admit to
being congested well before it has consumed a significant fraction of
memory in its output queue.

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

Thanks,
NeilBrown

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


#1451305 — Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-07-27 16:30 +0200
SubjectRe: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<rZy0u-y1-13@gated-at.bofh.it>
In reply to#1451030

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
> 

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


#1451463 — Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromMichal Hocko <mhocko@kernel.org>
Date2016-07-27 20:50 +0200
SubjectRe: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<rZC41-39n-23@gated-at.bofh.it>
In reply to#1451305
On Wed 27-07-16 10:28:40, Mikulas Patocka wrote:
> 
> 
> 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'm not really sure this is the right approach. If a particular mempool
user cannot ever be throttled by the page allocator then it should
perform GFP_NOWAIT. Even mempool allocations shouldn't allow reclaim to
scan pages too quickly even when LRU lists are full of dirty pages. But
as I've said that would restrict the success rates even under light page
cache load. Throttling on the wait_iff_congested should be quite rare.

Anyway do you see an excessive throttling with the patch posted
http://lkml.kernel.org/r/20160725192344.GD2166@dhcp22.suse.cz ? Or from
another side. Do you see an excessive number of dirty/writeback pages
wrt. the dirty threshold or any other undesirable side effects?
-- 
Michal Hocko
SUSE Labs

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


#1455827 — Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-08-03 16:30 +0200
SubjectRe: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<s25lg-3EJ-27@gated-at.bofh.it>
In reply to#1451463

On Wed, 27 Jul 2016, Michal Hocko wrote:

> On Wed 27-07-16 10:28:40, Mikulas Patocka wrote:
> > 
> > 
> > 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'm not really sure this is the right approach. If a particular mempool
> user cannot ever be throttled by the page allocator then it should
> perform GFP_NOWAIT.

Then, all block device drivers should have GFP_NOWAIT - which means that 
we can as well make it default.

But GFP_NOWAIT also disables direct reclaim. We really want direct reclaim 
when allocating from mempool - we just don't want to throttle due to block 
device congestion.

We could use __GFP_NORETRY as an indication that we don't want to sleep - 
or make a new flag __GFP_NO_THROTTLE.

> Even mempool allocations shouldn't allow reclaim to
> scan pages too quickly even when LRU lists are full of dirty pages. But
> as I've said that would restrict the success rates even under light page
> cache load. Throttling on the wait_iff_congested should be quite rare.
> 
> Anyway do you see an excessive throttling with the patch posted
> http://lkml.kernel.org/r/20160725192344.GD2166@dhcp22.suse.cz ? Or from

It didn't have much effect.

Since the patch 4e390b2b2f34b8daaabf2df1df0cf8f798b87ddb (revert of the 
limitless mempool allocations), swapping to dm-crypt works in the simple 
example.

> another side. Do you see an excessive number of dirty/writeback pages
> wrt. the dirty threshold or any other undesirable side effects?
> -- 
> Michal Hocko
> SUSE Labs

I also got got dmcrypt stalled in bt_get when submitting I/Os to the 
underlying virtio device. I don't know what could be done about it.

[   30.441074] dmcrypt_write   D ffff88003de7bba8     0  2155      2 0x00080000
[   30.441956]  ffff88003de7bba8 ffff88003de7be70 ffff88003de7c000 ffff88003fc34740
[   30.442934]  7fffffffffffffff ffff88003fc3a680 ffff880037a911f8 ffff88003de7bbc0
[   30.443969]  ffffffff812770df 7fffffffffffffff ffff88003de7bc10 ffffffff81278ca7
[   30.444926] Call Trace:
[   30.445232]  [<ffffffff812770df>] schedule+0x83/0x98
[   30.445825]  [<ffffffff81278ca7>] schedule_timeout+0x2f/0xcf
[   30.446506]  [<ffffffff81276c84>] io_schedule_timeout+0x64/0x90
[   30.447235]  [<ffffffff81276c84>] ? io_schedule_timeout+0x64/0x90
[   30.448088]  [<ffffffff8115787a>] bt_get+0x11a/0x1bc
[   30.448688]  [<ffffffff8105ef86>] ? wake_up_atomic_t+0x25/0x25
[   30.449392]  [<ffffffff81157abb>] blk_mq_get_tag+0x7e/0x9b
[   30.450041]  [<ffffffff81155066>] __blk_mq_alloc_request+0x1b/0x1e0
[   30.450805]  [<ffffffff81155ee8>] blk_mq_map_request+0xf6/0x136
[   30.451516]  [<ffffffff81156866>] blk_sq_make_request+0xac/0x173
[   30.452322]  [<ffffffff8114db56>] generic_make_request+0xb8/0x15b
[   30.453038]  [<ffffffffa012ba65>] dmcrypt_write+0x13b/0x174 [dm_crypt]
[   30.453852]  [<ffffffff81052779>] ? wake_up_q+0x42/0x42
[   30.454508]  [<ffffffffa012b92a>] ? crypt_iv_tcw_dtr+0x62/0x62 [dm_crypt]
[   30.455369]  [<ffffffff8104dc6a>] kthread+0xa0/0xa8
[   30.456041]  [<ffffffff8104dc6a>] ? kthread+0xa0/0xa8
[   30.456688]  [<ffffffff8127999f>] ret_from_fork+0x1f/0x40
[   30.457396]  [<ffffffff8104dbca>] ? init_completion+0x24/0x24

Mikulas

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


#1455834 — Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromMichal Hocko <mhocko@kernel.org>
Date2016-08-03 16:50 +0200
SubjectRe: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<s25EB-3Lb-23@gated-at.bofh.it>
In reply to#1455827
On Wed 03-08-16 09:59:11, Mikulas Patocka wrote:
> 
> 
> On Wed, 27 Jul 2016, Michal Hocko wrote:
> 
> > On Wed 27-07-16 10:28:40, Mikulas Patocka wrote:
[...]
> > > I think that approach with PF_LESS_THROTTLE in mempool_alloc is incorrect 
> > > and that mempool allocations should never be throttled.
> > 
> > I'm not really sure this is the right approach. If a particular mempool
> > user cannot ever be throttled by the page allocator then it should
> > perform GFP_NOWAIT.
> 
> Then, all block device drivers should have GFP_NOWAIT - which means that 
> we can as well make it default.
> 
> But GFP_NOWAIT also disables direct reclaim. We really want direct reclaim 
> when allocating from mempool - we just don't want to throttle due to block 
> device congestion.
> 
> We could use __GFP_NORETRY as an indication that we don't want to sleep - 
> or make a new flag __GFP_NO_THROTTLE.

__GFP_NORETRY is used for other contexts so it is not suitable.
__GFP_NO_THROTTLE would be possible but I would still prefer if we
didn't go that way unless really necessary.

> > Even mempool allocations shouldn't allow reclaim to
> > scan pages too quickly even when LRU lists are full of dirty pages. But
> > as I've said that would restrict the success rates even under light page
> > cache load. Throttling on the wait_iff_congested should be quite rare.
> > 
> > Anyway do you see an excessive throttling with the patch posted
> > http://lkml.kernel.org/r/20160725192344.GD2166@dhcp22.suse.cz ? Or from
> 
> It didn't have much effect.
> 
> Since the patch 4e390b2b2f34b8daaabf2df1df0cf8f798b87ddb (revert of the 
> limitless mempool allocations), swapping to dm-crypt works in the simple 
> example.

OK. Do you see any throttling due to wait_iff_congested?
writeback_wait_iff_congested trace point should help here. If not maybe
we should start with the above patch and see how it works in practise.
If the there is still an excessive and unexpected throttling then we
should move on to a more mempool/block layer users specific solution.
-- 
Michal Hocko
SUSE Labs

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


#1456628 — Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromMikulas Patocka <mpatocka@redhat.com>
Date2016-08-04 20:50 +0200
SubjectRe: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<s2vSp-4OE-3@gated-at.bofh.it>
In reply to#1455834

On Wed, 3 Aug 2016, Michal Hocko wrote:

> > > Even mempool allocations shouldn't allow reclaim to
> > > scan pages too quickly even when LRU lists are full of dirty pages. But
> > > as I've said that would restrict the success rates even under light page
> > > cache load. Throttling on the wait_iff_congested should be quite rare.
> > > 
> > > Anyway do you see an excessive throttling with the patch posted
> > > http://lkml.kernel.org/r/20160725192344.GD2166@dhcp22.suse.cz ? Or from
> > 
> > It didn't have much effect.
> > 
> > Since the patch 4e390b2b2f34b8daaabf2df1df0cf8f798b87ddb (revert of the 
> > limitless mempool allocations), swapping to dm-crypt works in the simple 
> > example.
> 
> OK. Do you see any throttling due to wait_iff_congested?

No, but I've seen occasional stalls of mempool allocations in 
throttle_vm_writeout - but the patch that removed throttle_vm_writeout 
didn't improve overall speed, so the stalls were only minor.

> writeback_wait_iff_congested trace point should help here. If not maybe
> we should start with the above patch and see how it works in practise.
> If the there is still an excessive and unexpected throttling then we
> should move on to a more mempool/block layer users specific solution.

Currently, dm-crypt reports the device congested only if the underlying 
block device is congested.

But as others suggested, dm-crypt should report congested status if is 
clogged due to slow encryption progress - and in that case you should not 
throttle mempool allocations (because such throttling would decrease 
encryption speed even more).

Mikulas

> -- 
> Michal Hocko
> SUSE Labs

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


#1451528 — Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks

FromNeilBrown <neilb@suse.com>
Date2016-07-27 23:40 +0200
SubjectRe: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Message-ID<rZEIy-4S2-3@gated-at.bofh.it>
In reply to#1451305

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jul 28 2016, Mikulas Patocka wrote:

> 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 very much agree with that last statement!  It may be that to get to
that point we will need all backing devices to signal congestion
correctly.

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

Thanks!  I'll have a look.

NeilBrown

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web