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

From: Michal Hocko <mhocko@kernel.org>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Mel Gorman <mgorman@suse.de>, NeilBrown <neilb@suse.com>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org,
	"dm-devel@redhat.com David Rientjes" <rientjes@google.com>,
	Ondrej Kozina <okozina@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [dm-devel] [RFC PATCH 2/2] mm, mempool: do not throttle PF_LESS_THROTTLE tasks
Date: Sun, 14 Aug 2016 12:34:09 +0200	[thread overview]
Message-ID: <20160814103409.GC9248@dhcp22.suse.cz> (raw)
In-Reply-To: <alpine.LRH.2.02.1608131323550.3291@file01.intranet.prod.int.rdu2.redhat.com>

On Sat 13-08-16 13:34:29, Mikulas Patocka wrote:
> 
> 
> On Fri, 12 Aug 2016, Michal Hocko wrote:
> 
> > On Thu 04-08-16 14:49:41, Mikulas Patocka wrote:
> > 
> > > On Wed, 3 Aug 2016, Michal Hocko wrote:
> > > 
> > > > But the device congestion is not the only condition required for the
> > > > throttling. The pgdat has also be marked congested which means that the
> > > > LRU page scanner bumped into dirty/writeback/pg_reclaim pages at the
> > > > tail of the LRU. That should only happen if we are rotating LRUs too
> > > > quickly. AFAIU the reclaim shouldn't allow free ticket scanning in that
> > > > situation.
> > > 
> > > The obvious problem here is that mempool allocations should sleep in 
> > > mempool_alloc() on &pool->wait (until someone returns some entries into 
> > > the mempool), they should not sleep inside the page allocator.
> > 
> > I agree that mempool_alloc should _primarily_ sleep on their own
> > throttling mechanism. I am not questioning that. I am just saying that
> > the page allocator has its own throttling which it relies on and that
> > cannot be just ignored because that might have other undesirable side
> > effects. So if the right approach is really to never throttle certain
> > requests then we have to bail out from a congested nodes/zones as soon
> > as the congestion is detected.
> > 
> > Now, I would like to see that something like that is _really_ necessary.
> 
> Currently, it is not a problem - device mapper reports the device as 
> congested only if the underlying physical disks are congested.
> 
> But once we change it so that device mapper reports congested state on its 
> own (when it has too many bios in progress), this starts being a problem.

OK, can we wait until it starts becoming a real problem and solve it
appropriately then?

I will repost the patch which removes thottle_vm_pageout in the meantime
as it doesn't seem to be needed anymore.

-- 
Michal Hocko
SUSE Labs