Re: [RFC PATCH 2/2] ext4: improve extents status tree shrinker to avoid scanning delayed entries

[Jan Kara <jack@suse.cz>]

On Wed 25-12-13 11:34:48, Zheng Liu wrote:
> On Mon, Dec 23, 2013 at 09:54:19AM +0100, Jan Kara wrote:
> > On Fri 20-12-13 18:42:45, Zheng Liu wrote:
> > > From: Zheng Liu <wenqing.lz@taobao.com>
> > > 
> > > The extents status tree shrinker will scan all inodes on sbi->s_es_lru
> > > under heavy memory pressure, and try to reclaim the entry from extents
> > > status tree.  During this process it couldn't reclaim the delayed entry
> > > because ext4 needs to use these entries to do delayed allocation space
> > > reservation, seek_data/hole, etc....  So if a system has done a huge
> > > number of writes and these dirty pages don't be written out.  There will
> > > be a lot of delayed entries on extents status tree.  If shrinker tries
> > > to reclaim memory from the tree, it will burn some CPU time to iterate
> > > on these non-reclaimable entries.  At some circumstances it could cause
> > > excessive stall time.
> > > 
> > > In this commit a new list is used to track reclaimable entries of extent
> > > status tree (e.g. written/unwritten/hole entries).  The shrinker will
> > > scan reclaimable entry on this list.  So it won't encouter any delayed
> > > entry and don't need to take too much time to spin.  But the defect is
> > > that we need to cost extra 1/3 memory space for one entry.  Before this
> > > commit, 'struct extent_status' occupies 48 bytes on a 64bits platform.
> > > After that it will occupy 64 bytes. :(
> >   This looks sensible. I was just wondering about one thing: One incorrect
> > thing the old extent shrinker does is that it tries to reclaim 'nr_to_scan'
> > objects. That is wrong - it should *scan* 'nr_to_scan' objects and reclaim
> > objects it can find. Now we shouldn't always start scanning at the end of
> > the LRU because if delayed extents accumulate there we would never reclaim
> > anything. Rather we should cycle through the list of entries we have. But
> > that doesn't play well with the fact we have LRU list and thus want to
> > reclaim from the end of the list. In the end what you do might be the best
> > we can do but I wanted to mention the above just in case someone has some
> > idea.
> 
> Ah, thanks for pointing it out.  So maybe we can fix this issue before
> we are sure that the new improvement is acceptable because it makes us
> avoid scanning too many objects.  What do you think?
  I'm sorry but I'm not sure I understand.  By 'fix this issue' do you mean
using your patch or somehow fixing the problem that we try to reclaim
'nr_to_scan' objects instead of just trying to scan that many objects?

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR