linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Wu Fengguang <fengguang.wu@intel.com>
Cc: Jan Kara <jack@suse.cz>, Rik van Riel <riel@redhat.com>,
	Greg Thelen <gthelen@google.com>,
	"bsingharora@gmail.com" <bsingharora@gmail.com>,
	Hugh Dickins <hughd@google.com>, Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, Mel Gorman <mgorman@suse.de>,
	Ying Han <yinghan@google.com>,
	"hannes@cmpxchg.org" <hannes@cmpxchg.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Minchan Kim <minchan.kim@gmail.com>
Subject: Re: reclaim the LRU lists full of dirty/writeback pages
Date: Mon, 20 Feb 2012 15:00:10 +0100	[thread overview]
Message-ID: <20120220140010.GF6799@quack.suse.cz> (raw)
In-Reply-To: <20120217164133.GA4871@localhost>

On Sat 18-02-12 00:41:33, Wu Fengguang wrote:
> > > > And I find the pageout works seem to have some problems with ext4.
> > > > For example, this can be easily triggered with 10 dd tasks running
> > > > inside the 100MB limited memcg:
> > >   So journal thread is getting stuck while committing transaction. Most
> > > likely waiting for some dd thread to stop a transaction so that commit can
> > > proceed. The processes waiting in start_this_handle() are just secondary
> > > effect resulting from the first problem. It might be interesting to get
> > > stack traces of all bloked processes when the journal thread is stuck.
> > 
> > For completeness of discussion, citing your conclusion on my private
> > data feed:
> > 
> > : We enter memcg reclaim from grab_cache_page_write_begin() and are
> > : waiting in congestion_wait(). Because grab_cache_page_write_begin() is
> > : called with transaction started, this blocks transaction from
> > : committing and subsequently blocks all other activity on the
> > : filesystem. The fact is this isn't new with your patches, just your
> > : changes or the fact that we are running in a memory constrained cgroup
> > : make this more visible.
> 
> Maybe I'm missing some deep FS restrictions, but can this page
> allocation (and the one in ext4_write_begin) be moved before
> ext4_journal_start()? So that the page reclaim can throttle the
> __GFP_WRITE allocations at will.
  You are missing the fact that in this way, things would deadlock quickly.
Lock ordering of ext4 is 'transaction start' -> 'page lock' (so that
writepages may be efficient) and so you cannot really have it differently
here - you can think of transaction start - transaction end pair as a
lock - unlock for lock ordering purposes. You could play some tricks like
allocating the page without locking, starting a transaction, locking a page
and checking that everything is OK but it isn't really nice and when memory
pressure is big, the gain is questionable.

								Honza
> ---
>  fs/ext4/inode.c |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> --- linux.orig/fs/ext4/inode.c	2012-02-18 00:10:27.000000000 +0800
> +++ linux/fs/ext4/inode.c	2012-02-18 00:31:19.000000000 +0800
> @@ -2398,38 +2398,38 @@ static int ext4_da_write_begin(struct fi
>  	if (ext4_nonda_switch(inode->i_sb)) {
>  		*fsdata = (void *)FALL_BACK_TO_NONDELALLOC;
>  		return ext4_write_begin(file, mapping, pos,
>  					len, flags, pagep, fsdata);
>  	}
>  	*fsdata = (void *)0;
>  	trace_ext4_da_write_begin(inode, pos, len, flags);
>  retry:
> +	page = grab_cache_page_write_begin(mapping, index, flags);
> +	if (!page) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	*pagep = page;
> +
>  	/*
>  	 * With delayed allocation, we don't log the i_disksize update
>  	 * if there is delayed block allocation. But we still need
>  	 * to journalling the i_disksize update if writes to the end
>  	 * of file which has an already mapped buffer.
>  	 */
>  	handle = ext4_journal_start(inode, 1);
>  	if (IS_ERR(handle)) {
>  		ret = PTR_ERR(handle);
> +		unlock_page(page);
> +		page_cache_release(page);
> +		if (pos + len > inode->i_size)
> +			truncate_inode_pages(inode->i_mapping, inode->i_size);
>  		goto out;
>  	}
> -	/* We cannot recurse into the filesystem as the transaction is already
> -	 * started */
> -	flags |= AOP_FLAG_NOFS;
> -
> -	page = grab_cache_page_write_begin(mapping, index, flags);
> -	if (!page) {
> -		ext4_journal_stop(handle);
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> -	*pagep = page;
>  
>  	ret = __block_write_begin(page, pos, len, ext4_da_get_block_prep);
>  	if (ret < 0) {
>  		unlock_page(page);
>  		ext4_journal_stop(handle);
>  		page_cache_release(page);
>  		/*
>  		 * block_write_begin may have instantiated a few blocks
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-02-20 14:00 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-08  7:55 memcg writeback (was Re: [Lsf-pc] [LSF/MM TOPIC] memcg topics.) Greg Thelen
2012-02-08  9:31 ` Wu Fengguang
2012-02-08 20:54   ` Ying Han
2012-02-09 13:50     ` Wu Fengguang
2012-02-13 18:40       ` Ying Han
2012-02-10  5:51   ` Greg Thelen
2012-02-10  5:52     ` Greg Thelen
2012-02-10  9:20       ` Wu Fengguang
2012-02-10 11:47     ` Wu Fengguang
2012-02-11 12:44       ` reclaim the LRU lists full of dirty/writeback pages Wu Fengguang
2012-02-11 14:55         ` Rik van Riel
2012-02-12  3:10           ` Wu Fengguang
2012-02-12  6:45             ` Wu Fengguang
2012-02-13 15:43             ` Jan Kara
2012-02-14 10:03               ` Wu Fengguang
2012-02-14 13:29                 ` Jan Kara
2012-02-16  4:00                   ` Wu Fengguang
2012-02-16 12:44                     ` Jan Kara
2012-02-16 13:32                       ` Wu Fengguang
2012-02-16 14:06                         ` Wu Fengguang
2012-02-17 16:41                     ` Wu Fengguang
2012-02-20 14:00                       ` Jan Kara [this message]
2012-02-14 10:19         ` Mel Gorman
2012-02-14 13:18           ` Wu Fengguang
2012-02-14 13:35             ` Wu Fengguang
2012-02-14 15:51             ` Mel Gorman
2012-02-16  9:50               ` Wu Fengguang
2012-02-16 17:31                 ` Mel Gorman
2012-02-27 14:24                   ` Fengguang Wu
2012-02-16  0:00             ` KAMEZAWA Hiroyuki
2012-02-16  3:04               ` Wu Fengguang
2012-02-16  3:52                 ` KAMEZAWA Hiroyuki
2012-02-16  4:05                   ` Wu Fengguang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120220140010.GF6799@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=bsingharora@gmail.com \
    --cc=fengguang.wu@intel.com \
    --cc=gthelen@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --cc=minchan.kim@gmail.com \
    --cc=riel@redhat.com \
    --cc=yinghan@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).