Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1450036 > unrolled thread
| Started by | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| First post | 2016-07-26 00:00 +0200 |
| Last post | 2016-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.
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
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-26 00:00 +0200 |
| Subject | Re: [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]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-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]
| From | NeilBrown <neilb@suse.com> |
|---|---|
| Date | 2016-07-27 06:10 +0200 |
| Subject | Re: [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]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-07-27 16:30 +0200 |
| Subject | Re: [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]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-07-27 20:50 +0200 |
| Subject | Re: [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]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-08-03 16:30 +0200 |
| Subject | Re: [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]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-08-03 16:50 +0200 |
| Subject | Re: [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]
| From | Mikulas Patocka <mpatocka@redhat.com> |
|---|---|
| Date | 2016-08-04 20:50 +0200 |
| Subject | Re: [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]
| From | NeilBrown <neilb@suse.com> |
|---|---|
| Date | 2016-07-27 23:40 +0200 |
| Subject | Re: [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