Prev: linux-next: build failure after merge of the final tree (xen tree related)
Next: writeback: the kupdate expire timestamp should be a moving target
From: Mel Gorman on 26 Jul 2010 07:00
On Thu, Jul 22, 2010 at 01:09:32PM +0800, Wu Fengguang wrote:
> A background flush work may run for ever. So it's reasonable for it to
> mimic the kupdate behavior of syncing old/expired inodes first.
>
> The policy is
> - enqueue all newly expired inodes at each queue_io() time
> - retry with halfed expire interval until get some inodes to sync
>
> CC: Jan Kara <jack(a)suse.cz>
> Signed-off-by: Wu Fengguang <fengguang.wu(a)intel.com>

Ok, intuitively this would appear to tie into pageout where we want
older inodes to be cleaned first by background flushers to limit the
number of dirty pages encountered by page reclaim. If this is accurate,
it should be detailed in the changelog.

> ---
> fs/fs-writeback.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> --- linux-next.orig/fs/fs-writeback.c 2010-07-22 12:56:42.000000000 +0800
> +++ linux-next/fs/fs-writeback.c 2010-07-22 13:07:51.000000000 +0800
> @@ -217,14 +217,14 @@ static void move_expired_inodes(struct l
> struct writeback_control *wbc)
> {
> unsigned long expire_interval = 0;
> - unsigned long older_than_this;
> + unsigned long older_than_this = 0; /* reset to kill gcc warning */
> LIST_HEAD(tmp);
> struct list_head *pos, *node;
> struct super_block *sb = NULL;
> struct inode *inode;
> int do_sb_sort = 0;
>
> - if (wbc->for_kupdate) {
> + if (wbc->for_kupdate || wbc->for_background) {
> expire_interval = msecs_to_jiffies(dirty_expire_interval * 10);
> older_than_this = jiffies - expire_interval;
> }
> @@ -232,8 +232,15 @@ static void move_expired_inodes(struct l
> while (!list_empty(delaying_queue)) {
> inode = list_entry(delaying_queue->prev, struct inode, i_list);
> if (expire_interval &&
> - inode_dirtied_after(inode, older_than_this))
> - break;
> + inode_dirtied_after(inode, older_than_this)) {
> + if (wbc->for_background &&
> + list_empty(dispatch_queue) && list_empty(&tmp)) {
> + expire_interval >>= 1;
> + older_than_this = jiffies - expire_interval;
> + continue;
> + } else
> + break;
> + }

This needs a comment.

I think what it is saying is that if background flush is active but no
inodes are old enough, consider newer inodes. This is on the assumption
that page reclaim has encountered dirty pages and the dirty inodes are
still too young.

> if (sb && sb != inode->i_sb)
> do_sb_sort = 1;
> sb = inode->i_sb;
> @@ -521,7 +528,8 @@ void writeback_inodes_wb(struct bdi_writ
>
> wbc->wb_start = jiffies; /* livelock avoidance */
> spin_lock(&inode_lock);
> - if (!wbc->for_kupdate || list_empty(&wb->b_io))
> +
> + if (!(wbc->for_kupdate || wbc->for_background) || list_empty(&wb->b_io))
> queue_io(wb, wbc);
>
> while (!list_empty(&wb->b_io)) {
> @@ -550,7 +558,7 @@ static void __writeback_inodes_sb(struct
>
> wbc->wb_start = jiffies; /* livelock avoidance */
> spin_lock(&inode_lock);
> - if (!wbc->for_kupdate || list_empty(&wb->b_io))
> + if (!(wbc->for_kupdate || wbc->for_background) || list_empty(&wb->b_io))
> queue_io(wb, wbc);
> writeback_sb_inodes(sb, wb, wbc, true);
> spin_unlock(&inode_lock);
>
>

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mel Gorman on 26 Jul 2010 09:10
On Mon, Jul 26, 2010 at 08:56:35PM +0800, Wu Fengguang wrote:
> > > @@ -232,8 +232,15 @@ static void move_expired_inodes(struct l
> > > while (!list_empty(delaying_queue)) {
> > > inode = list_entry(delaying_queue->prev, struct inode, i_list);
> > > if (expire_interval &&
> > > - inode_dirtied_after(inode, older_than_this))
> > > - break;
> > > + inode_dirtied_after(inode, older_than_this)) {
> > > + if (wbc->for_background &&
> > > + list_empty(dispatch_queue) && list_empty(&tmp)) {
> > > + expire_interval >>= 1;
> > > + older_than_this = jiffies - expire_interval;
> > > + continue;
> > > + } else
> > > + break;
> > > + }
> >
> > This needs a comment.
> >
> > I think what it is saying is that if background flush is active but no
> > inodes are old enough, consider newer inodes. This is on the assumption
> > that page reclaim has encountered dirty pages and the dirty inodes are
> > still too young.
>
> Yes this should be commented. How about this one?
>
> @@ -232,8 +232,20 @@ static void move_expired_inodes(struct l
> while (!list_empty(delaying_queue)) {
> inode = list_entry(delaying_queue->prev, struct inode, i_list);
> if (expire_interval &&
> - inode_dirtied_after(inode, older_than_this))
> + inode_dirtied_after(inode, older_than_this)) {
> + /*
> + * background writeback will start with expired inodes,
> + * and then fresh inodes. This order helps reducing
> + * the number of dirty pages reaching the end of LRU
> + * lists and cause trouble to the page reclaim.
> + */

s/reducing/reduce/

Otherwise, it's enough detail to know what is going on. Thanks

Thanks

> + if (wbc->for_background &&
> + list_empty(dispatch_queue) && list_empty(&tmp)) {
> + expire_interval = 0;
> + continue;
> + }
> break;
> + }
> if (sb && sb != inode->i_sb)
> do_sb_sort = 1;
> sb = inode->i_sb;
>

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mel Gorman on 27 Jul 2010 05:50
On Mon, Jul 26, 2010 at 09:11:52PM +0800, Wu Fengguang wrote:
> On Mon, Jul 26, 2010 at 08:59:55PM +0800, Mel Gorman wrote:
> > On Mon, Jul 26, 2010 at 08:56:35PM +0800, Wu Fengguang wrote:
> > > > > @@ -232,8 +232,15 @@ static void move_expired_inodes(struct l
> > > > > while (!list_empty(delaying_queue)) {
> > > > > inode = list_entry(delaying_queue->prev, struct inode, i_list);
> > > > > if (expire_interval &&
> > > > > - inode_dirtied_after(inode, older_than_this))
> > > > > - break;
> > > > > + inode_dirtied_after(inode, older_than_this)) {
> > > > > + if (wbc->for_background &&
> > > > > + list_empty(dispatch_queue) && list_empty(&tmp)) {
> > > > > + expire_interval >>= 1;
> > > > > + older_than_this = jiffies - expire_interval;
> > > > > + continue;
> > > > > + } else
> > > > > + break;
> > > > > + }
> > > >
> > > > This needs a comment.
> > > >
> > > > I think what it is saying is that if background flush is active but no
> > > > inodes are old enough, consider newer inodes. This is on the assumption
> > > > that page reclaim has encountered dirty pages and the dirty inodes are
> > > > still too young.
> > >
> > > Yes this should be commented. How about this one?
> > >
> > > @@ -232,8 +232,20 @@ static void move_expired_inodes(struct l
> > > while (!list_empty(delaying_queue)) {
> > > inode = list_entry(delaying_queue->prev, struct inode, i_list);
> > > if (expire_interval &&
> > > - inode_dirtied_after(inode, older_than_this))
> > > + inode_dirtied_after(inode, older_than_this)) {
> > > + /*
> > > + * background writeback will start with expired inodes,
> > > + * and then fresh inodes. This order helps reducing
> > > + * the number of dirty pages reaching the end of LRU
> > > + * lists and cause trouble to the page reclaim.
> > > + */
> >
> > s/reducing/reduce/
> >
> > Otherwise, it's enough detail to know what is going on. Thanks
>
> Thanks. Here is the updated patch.
> ---
> Subject: writeback: sync expired inodes first in background writeback
> From: Wu Fengguang <fengguang.wu(a)intel.com>
> Date: Wed Jul 21 20:11:53 CST 2010
>
> A background flush work may run for ever. So it's reasonable for it to
> mimic the kupdate behavior of syncing old/expired inodes first.
>
> The policy is
> - enqueue all newly expired inodes at each queue_io() time
> - enqueue all dirty inodes if there are no more expired inodes to sync
>
> This will help reduce the number of dirty pages encountered by page
> reclaim, eg. the pageout() calls. Normally older inodes contain older
> dirty pages, which are more close to the end of the LRU lists. So
> syncing older inodes first helps reducing the dirty pages reached by
> the page reclaim code.
>
> CC: Jan Kara <jack(a)suse.cz>
> Signed-off-by: Wu Fengguang <fengguang.wu(a)intel.com>

Acked-by: Mel Gorman <mel(a)csn.ul.ie>

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: linux-next: build failure after merge of the final tree (xen tree related)
Next: writeback: the kupdate expire timestamp should be a moving target