writeback: try to write older pages first
Subsystems (7)
cover writeback: try to write older pages first
Andrew,
The basic way of avoiding pageout() is to make the flusher sync inodes in the right order. Oldest dirty inodes contains oldest pages. The smaller inode it is, the more correlation between inode dirty time and its pages' dirty time. So for small dirty inodes, syncing in the order of inode dirty time is able to avoid pageout(). If pageout() is still triggered frequently in this case, the 30s dirty expire time may be too long and could be shrinked adaptively; or it may be a stressed memcg list whose dirty inodes/pages are more hard to track.
For a large dirty inode, it may flush lots of newly dirtied pages _after_ syncing the expired pages. This is the normal case for a single-stream sequential dirtier, where older pages are in lower offsets. In this case we shall not insist on syncing the whole large dirty inode before considering the other small dirty inodes. This risks wasting time syncing 1GB freshly dirtied pages before syncing the other N*1MB expired dirty pages who are approaching the end of the LRU list and hence pageout().
For a large dirty inode, it may also flush lots of newly dirtied pages _before_ hitting the desired old ones, in which case it helps for pageout() to do some clustered writeback, and/or set mapping->writeback_index to help the flusher focus on old pages.
For a large dirty inode, it may also have intermixed old and new dirty pages. In this case we need to make sure the inode is queued for IO before some of its pages hit pageout(). Adaptive dirty expire time helps here.
OK, end of the vapour ideas. As for this patchset, it fixes the current kupdate/background writeback priority:
- the kupdate/background writeback shall include newly expired inodes at each queue_io() time, as the large inodes left over from previous writeback rounds are likely to have less density of old pages.
- the background writeback shall consider expired inodes first, just like the kupdate writeback
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 1/6 writeback: pass writeback_control down to move_expired_inodes()
This is to prepare for moving the dirty expire policy to move_expired_inodes(). No behavior change.
--- fs/fs-writeback.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
fs/fs-writeback.c +8 -8
@@ -213,8 +213,8 @@ static bool inode_dirtied_after(struct i* Move expired dirty inodes from @delaying_queue to @dispatch_queue. */ static void move_expired_inodes (struct list_head *delaying_queue, -struct list_head *dispatch_queue, -unsigned long *older_than_this) +struct list_head *dispatch_queue, +struct writeback_control *wbc) {LIST_HEAD (tmp);struct list_head *pos, *node;
@@ -224,8 +224,8 @@ static void move_expired_inodes(struct lwhile (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); -if (older_than_this && -inode_dirtied_after (inode, *older_than_this)) +if (wbc->older_than_this && +inode_dirtied_after (inode, *wbc->older_than_this))break ;if (sb && sb != inode->i_sb) do_sb_sort =1 ;
@@ -257,10 +257,10 @@ static void move_expired_inodes(struct l* => b_more_io inodes * => remaining inodes in b_io => (dequeue for sync) */ -static void queue_io (struct bdi_writeback *wb,unsigned long *older_than_this) +static void queue_io (struct bdi_writeback *wb,struct writeback_control *wbc) {list_splice_init (&wb->b_more_io, &wb->b_io); -move_expired_inodes (&wb->b_dirty, &wb->b_io, older_than_this); +move_expired_inodes (&wb->b_dirty, &wb->b_io, wbc); }static int write_inode (struct inode *inode,struct writeback_control *wbc)
@@ -519,7 +519,7 @@ 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)) -queue_io (wb, wbc->older_than_this); +queue_io (wb, wbc);while (!list_empty (&wb->b_io)) { struct inode *inode = list_entry(wb->b_io.prev,
@@ -548,7 +548,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)) -queue_io (wb, wbc->older_than_this); +queue_io (wb, wbc);writeback_sb_inodes (sb, wb, wbc, true);spin_unlock (&inode_lock); }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/6] writeback: pass writeback_control down to move_expired_inodes()
Quoted 2 lines · Wu Fengguang
This is to prepare for moving the dirty expire policy to
move_expired_inodes(). No behavior change.
Looks OK.
Quoted 66 lines
Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)fs/fs-writeback.c +8
-9@@ -213,8 +213,8 @@ static bool inode_dirtied_after(struct i* Move expired dirty inodes from @delaying_queue to @dispatch_queue. */ static void move_expired_inodes (struct list_head *delaying_queue, -struct list_head *dispatch_queue, -unsigned long *older_than_this) +struct list_head *dispatch_queue, +struct writeback_control *wbc) {LIST_HEAD (tmp);struct list_head *pos, *node;@@ -224,8 +224,8 @@ static void move_expired_inodes(struct lwhile (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); -if (older_than_this && -inode_dirtied_after (inode, *older_than_this)) +if (wbc->older_than_this && +inode_dirtied_after (inode, *wbc->older_than_this))break ;if (sb && sb != inode->i_sb) do_sb_sort =1 ;@@ -257,10 +257,10 @@ static void move_expired_inodes(struct l* => b_more_io inodes * => remaining inodes in b_io => (dequeue for sync) */ -static void queue_io (struct bdi_writeback *wb,unsigned long *older_than_this) +static void queue_io (struct bdi_writeback *wb,struct writeback_control *wbc) {list_splice_init (&wb->b_more_io, &wb->b_io); -move_expired_inodes (&wb->b_dirty, &wb->b_io, older_than_this); +move_expired_inodes (&wb->b_dirty, &wb->b_io, wbc); }static int write_inode (struct inode *inode,struct writeback_control *wbc)@@ -519,7 +519,7 @@ 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)) -queue_io (wb, wbc->older_than_this); +queue_io (wb, wbc);while (!list_empty (&wb->b_io)) { struct inode *inode = list_entry(wb->b_io.prev,@@ -548,7 +548,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)) -queue_io (wb, wbc->older_than_this); +queue_io (wb, wbc);writeback_sb_inodes (sb, wb, wbc, true);spin_unlock (&inode_lock); } --To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/6] writeback: pass writeback_control down to move_expired_inodes()
In reply to Wu FengguangQuoted 4 lines · Wu Fengguang
This is to prepare for moving the dirty expire policy to
move_expired_inodes(). No behavior change.Signed-off-by Wu Fengguang <[email protected]>
Can't see any problem.
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/6] writeback: pass writeback_control down to move_expired_inodes()
In reply to Wu FengguangQuoted 4 lines · Wu Fengguang
This is to prepare for moving the dirty expire policy to
move_expired_inodes(). No behavior change.Signed-off-by Wu Fengguang <[email protected]>
Signature · 8 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/6 writeback: the kupdate expire timestamp should be a moving target
In reply to Wu FengguangDynamicly compute the dirty expire timestamp at queue_io() time. Also remove writeback_control.older_than_this which is no longer used.
writeback_control.older_than_this used to be determined at entrance to the kupdate writeback work. This _static_ timestamp may go stale if the kupdate work runs on and on. The flusher may then stuck with some old busy inodes, never considering newly expired inodes thereafter.
This has two possible problems:
- It is unfair for a large dirty inode to delay (for a long time) the writeback of small dirty inodes.
- As time goes by, the large and busy dirty inode may contain only _freshly_ dirtied pages. Ignoring newly expired dirty inodes risks delaying the expired dirty pages to the end of LRU lists, triggering the very bad pageout(). Neverthless this patch merely addresses part of the problem.
--- fs/fs-writeback.c | 24 +++++++++--------------- include/linux/writeback.h | 2 -- include/trace/events/writeback.h | 6 +----- mm/backing-dev.c | 1 - mm/page-writeback.c | 1 - 5 files changed, 10 insertions(+), 24 deletions(-)
fs/fs-writeback.c +9 -15
@@ -216,16 +216,23 @@ static void move_expired_inodes(struct lstruct list_head *dispatch_queue, struct writeback_control *wbc) { + unsigned long expire_interval = 0; +unsigned long older_than_this;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) { + expire_interval =msecs_to_jiffies (dirty_expire_interval *10 ); + older_than_this = jiffies - expire_interval; + } +while (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); -if (wbc->older_than_this && -inode_dirtied_after (inode, *wbc->older_than_this)) +if (expire_interval && +inode_dirtied_after (inode, older_than_this))break ;if (sb && sb != inode->i_sb) do_sb_sort =1 ;
@@ -583,29 +590,19 @@ static inline bool over_bground_thresh(v* Try to run once per dirty_writeback_interval. But if a writeback event * takes longer than a dirty_writeback_interval interval, then leave a * one-second gap. -* -* older_than_this takes precedence over nr_to_write. So we'll only write back -* all dirty pages if they are all attached to "old" mappings. */ static long wb_writeback (struct bdi_writeback *wb,struct wb_writeback_work *work) {struct writeback_control wbc = { .sync_mode = work->sync_mode, - .older_than_this =NULL , .for_kupdate = work->for_kupdate, .for_background = work->for_background, .range_cyclic = work->range_cyclic, }; -unsigned long oldest_jif;long wrote =0 ;struct inode *inode; -if (wbc.for_kupdate) { - wbc.older_than_this = &oldest_jif; - oldest_jif = jiffies - -msecs_to_jiffies (dirty_expire_interval *10 ); - }if (!wbc.range_cyclic) { wbc.range_start =0 ; wbc.range_end =LLONG_MAX ;
@@ -998,9 +995,6 @@ EXPORT_SYMBOL(__mark_inode_dirty);* Write out a superblock's list of dirty inodes. A wait will be performed * upon no inodes, all inodes or the final one, depending upon sync_mode. * -* If older_than_this is non-NULL, then only write out inodes which -* had their first dirtying at a time earlier than *older_than_this. -* * If `bdi' is non-zero then we're being asked to writeback a specific queue. * This function assumes that the blockdev superblock's inodes are backed by * a variety of queues, so all inodes are searched. For other superblocks,
include/linux/writeback.h +0 -2
@@ -28,8 +28,6 @@ enum writeback_sync_modes {*/ struct writeback_control {enum writeback_sync_modes sync_mode; -unsigned long *older_than_this;/* If !NULL, only write back inodes -older than this */ unsigned long wb_start;/* Time writeback_inodes_wb was called. This is needed to avoid extra jobs and livelock */
include/trace/events/writeback.h +1 -5
@@ -100,7 +100,6 @@ DECLARE_EVENT_CLASS(wbc_class, __field(int, for_reclaim) __field(int, range_cyclic) __field(int, more_io) - __field(unsigned long, older_than_this) __field(long, range_start)__field (long, range_end) ),
@@ -115,14 +114,12 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_reclaim = wbc->for_reclaim; __entry->range_cyclic = wbc->range_cyclic; __entry->more_io = wbc->more_io; - __entry->older_than_this = wbc->older_than_this ? - *wbc->older_than_this :0 ; __entry->range_start = (long )wbc->range_start; __entry->range_end = (long )wbc->range_end; ), TP_printk("bdi %s: towrt=%ld skip=%ld mode=%d kupd=%d " - "bgrd=%d reclm=%d cyclic=%d more=%d older=0x%lx " + "bgrd=%d reclm=%d cyclic=%d more=%d " "start=0x%lx end=0x%lx", __entry->name, __entry->nr_to_write,
@@ -133,7 +130,6 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_reclaim, __entry->range_cyclic, __entry->more_io, - __entry->older_than_this, __entry->range_start, __entry->range_end) )
mm/page-writeback.c +0 -1
@@ -482,7 +482,6 @@ static void balance_dirty_pages(struct afor (;;) {struct writeback_control wbc = { .sync_mode =WB_SYNC_NONE , - .older_than_this =NULL , .nr_to_write = write_chunk, .range_cyclic =1 , };
mm/backing-dev.c +0 -1
@@ -271,7 +271,6 @@ static void bdi_flush_io(struct backing_ {struct writeback_control wbc = { .sync_mode =WB_SYNC_NONE , - .older_than_this =NULL , .range_cyclic =1 , .nr_to_write =1024 , };
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 2/6] writeback: the kupdate expire timestamp should be a moving target
Quoted 7 lines · Wu Fengguang
Dynamicly compute the dirty expire timestamp at
queue_io()time. Also remove writeback_control.older_than_this which is no longer used.writeback_control.older_than_this used to be determined at entrance to the kupdate writeback work. This _static_ timestamp may go stale if the kupdate work runs on and on. The flusher may then stuck with some old busy inodes, never considering newly expired inodes thereafter.
This seems to make sense. The patch looks fine as well.
Honza
Quoted 161 lines
This has two possible problems:
- It is unfair for a large dirty inode to delay (for a long time) the writeback of small dirty inodes.
- As time goes by, the large and busy dirty inode may contain only _freshly_ dirtied pages. Ignoring newly expired dirty inodes risks delaying the expired dirty pages to the end of LRU lists, triggering the very bad
pageout(). Neverthless this patch merely addresses part of the problem.CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 24 +++++++++--------------- include/linux/writeback.h | 2 -- include/trace/events/writeback.h | 6 +----- mm/backing-dev.c | 1 - mm/page-writeback.c | 1 - 5 files changed, 10 insertions(+), 24 deletions(-)fs/fs-writeback.c +9
-15@@ -216,16 +216,23 @@ static void move_expired_inodes(struct lstruct list_head *dispatch_queue, struct writeback_control *wbc) { + unsigned long expire_interval = 0; +unsigned long older_than_this;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) { + expire_interval =msecs_to_jiffies (dirty_expire_interval *10 ); + older_than_this = jiffies - expire_interval; + } +while (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); -if (wbc->older_than_this && -inode_dirtied_after (inode, *wbc->older_than_this)) +if (expire_interval && +inode_dirtied_after (inode, older_than_this))break ;if (sb && sb != inode->i_sb) do_sb_sort =1 ;@@ -583,29 +590,19 @@ static inline bool over_bground_thresh(v* Try to run once per dirty_writeback_interval. But if a writeback event * takes longer than a dirty_writeback_interval interval, then leave a * one-second gap. -* -* older_than_this takes precedence over nr_to_write. So we'll only write back -* all dirty pages if they are all attached to "old" mappings. */ static long wb_writeback (struct bdi_writeback *wb,struct wb_writeback_work *work) {struct writeback_control wbc = { .sync_mode = work->sync_mode, - .older_than_this =NULL , .for_kupdate = work->for_kupdate, .for_background = work->for_background, .range_cyclic = work->range_cyclic, }; -unsigned long oldest_jif;long wrote =0 ;struct inode *inode; -if (wbc.for_kupdate) { - wbc.older_than_this = &oldest_jif; - oldest_jif = jiffies - -msecs_to_jiffies (dirty_expire_interval *10 ); - }if (!wbc.range_cyclic) { wbc.range_start =0 ; wbc.range_end =LLONG_MAX ;@@ -998,9 +995,6 @@ EXPORT_SYMBOL(__mark_inode_dirty);* Write out a superblock's list of dirty inodes. A wait will be performed * upon no inodes, all inodes or the final one, depending upon sync_mode. * -* If older_than_this is non-NULL, then only write out inodes which -* had their first dirtying at a time earlier than *older_than_this. -* * If `bdi' is non-zero then we're being asked to writeback a specific queue. * This function assumes that the blockdev superblock's inodes are backed by * a variety of queues, so all inodes are searched. For other superblocks, include/linux/writeback.h +0
-2@@ -28,8 +28,6 @@ enum writeback_sync_modes {*/ struct writeback_control {enum writeback_sync_modes sync_mode; -unsigned long *older_than_this;/* If !NULL, only write back inodes -older than this */ unsigned long wb_start;/* Time writeback_inodes_wb was called. This is needed to avoid extra jobs and livelock */ include/trace/events/writeback.h +1
-5@@ -100,7 +100,6 @@ DECLARE_EVENT_CLASS(wbc_class, __field(int, for_reclaim) __field(int, range_cyclic) __field(int, more_io) - __field(unsigned long, older_than_this) __field(long, range_start)__field (long, range_end) ),@@ -115,14 +114,12 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_reclaim = wbc->for_reclaim; __entry->range_cyclic = wbc->range_cyclic; __entry->more_io = wbc->more_io; - __entry->older_than_this = wbc->older_than_this ? - *wbc->older_than_this :0 ; __entry->range_start = (long )wbc->range_start; __entry->range_end = (long )wbc->range_end; ), TP_printk("bdi %s: towrt=%ld skip=%ld mode=%d kupd=%d " - "bgrd=%d reclm=%d cyclic=%d more=%d older=0x%lx " + "bgrd=%d reclm=%d cyclic=%d more=%d " "start=0x%lx end=0x%lx", __entry->name, __entry->nr_to_write,@@ -133,7 +130,6 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_reclaim, __entry->range_cyclic, __entry->more_io, - __entry->older_than_this, __entry->range_start, __entry->range_end) )mm/page-writeback.c +0
-1@@ -482,7 +482,6 @@ static void balance_dirty_pages(struct afor (;;) {struct writeback_control wbc = { .sync_mode =WB_SYNC_NONE , - .older_than_this =NULL , .nr_to_write = write_chunk, .range_cyclic =1 , };mm/backing-dev.c +0
-2@@ -271,7 +271,6 @@ static void bdi_flush_io(struct backing_ {struct writeback_control wbc = { .sync_mode =WB_SYNC_NONE , - .older_than_this =NULL , .range_cyclic =1 , .nr_to_write =1024 , }; --To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 2/6] writeback: the kupdate expire timestamp should be a moving target
In reply to Wu FengguangQuoted 13 lines · Wu Fengguang
Dynamicly compute the dirty expire timestamp at
queue_io()time. Also remove writeback_control.older_than_this which is no longer used.writeback_control.older_than_this used to be determined at entrance to the kupdate writeback work. This _static_ timestamp may go stale if the kupdate work runs on and on. The flusher may then stuck with some old busy inodes, never considering newly expired inodes thereafter.
This has two possible problems:
- It is unfair for a large dirty inode to delay (for a long time) the writeback of small dirty inodes.
Quoted 8 lines
- As time goes by, the large and busy dirty inode may contain only _freshly_ dirtied pages. Ignoring newly expired dirty inodes risks delaying the expired dirty pages to the end of LRU lists, triggering the very bad
pageout(). Neverthless this patch merely addresses part of the problem.CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>
Again, makes sense and I can't see a problem. There are some worth smithing issues in the changelog such as Dynamicly -> Dynamically and s/writeback_control.older_than_this used/writeback_control.older_than_this is used/ but other than that.
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 2/6] writeback: the kupdate expire timestamp should be a moving target
Quoted 20 lines · Mel Gorman
Quoted 19 lines · Wu Fengguang
Dynamicly compute the dirty expire timestamp at
queue_io()time. Also remove writeback_control.older_than_this which is no longer used.writeback_control.older_than_this used to be determined at entrance to the kupdate writeback work. This _static_ timestamp may go stale if the kupdate work runs on and on. The flusher may then stuck with some old busy inodes, never considering newly expired inodes thereafter.
This has two possible problems:
- It is unfair for a large dirty inode to delay (for a long time) the writeback of small dirty inodes.
- As time goes by, the large and busy dirty inode may contain only _freshly_ dirtied pages. Ignoring newly expired dirty inodes risks delaying the expired dirty pages to the end of LRU lists, triggering the very bad
pageout(). Neverthless this patch merely addresses part of the problem.
Quoted 5 lines
Quoted 2 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>Again, makes sense and I can't see a problem. There are some worth smithing issues in the changelog such as Dynamicly -> Dynamically and
Hah forgot to enable spell checking.
Quoted 1 line
s/writeback_control.older_than_this used/writeback_control.older_than_this is used/
It's "used to", my god.
Quoted 3 lines
but other than that.
Acked-by Mel Gorman <[email protected]>
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 2/6] writeback: the kupdate expire timestamp should be a moving target
In reply to Wu FengguangQuoted 100 lines · Wu Fengguang
Dynamicly compute the dirty expire timestamp at
queue_io()time. Also remove writeback_control.older_than_this which is no longer used.writeback_control.older_than_this used to be determined at entrance to the kupdate writeback work. This _static_ timestamp may go stale if the kupdate work runs on and on. The flusher may then stuck with some old busy inodes, never considering newly expired inodes thereafter.
This has two possible problems:
- It is unfair for a large dirty inode to delay (for a long time) the writeback of small dirty inodes.
- As time goes by, the large and busy dirty inode may contain only _freshly_ dirtied pages. Ignoring newly expired dirty inodes risks delaying the expired dirty pages to the end of LRU lists, triggering the very bad
pageout(). Neverthless this patch merely addresses part of the problem.CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 24 +++++++++--------------- include/linux/writeback.h | 2 -- include/trace/events/writeback.h | 6 +----- mm/backing-dev.c | 1 - mm/page-writeback.c | 1 - 5 files changed, 10 insertions(+), 24 deletions(-)fs/fs-writeback.c +9
-15@@ -216,16 +216,23 @@ static void move_expired_inodes(struct lstruct list_head *dispatch_queue, struct writeback_control *wbc) { + unsigned long expire_interval = 0; +unsigned long older_than_this;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) { + expire_interval =msecs_to_jiffies (dirty_expire_interval *10 ); + older_than_this = jiffies - expire_interval; + } +while (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); -if (wbc->older_than_this && -inode_dirtied_after (inode, *wbc->older_than_this)) +if (expire_interval && +inode_dirtied_after (inode, older_than_this))break ;if (sb && sb != inode->i_sb) do_sb_sort =1 ;@@ -583,29 +590,19 @@ static inline bool over_bground_thresh(v* Try to run once per dirty_writeback_interval. But if a writeback event * takes longer than a dirty_writeback_interval interval, then leave a * one-second gap. -* -* older_than_this takes precedence over nr_to_write. So we'll only write back -* all dirty pages if they are all attached to "old" mappings. */ static long wb_writeback (struct bdi_writeback *wb,struct wb_writeback_work *work) {struct writeback_control wbc = { .sync_mode = work->sync_mode, - .older_than_this =NULL , .for_kupdate = work->for_kupdate, .for_background = work->for_background, .range_cyclic = work->range_cyclic, }; -unsigned long oldest_jif;long wrote =0 ;struct inode *inode; -if (wbc.for_kupdate) { - wbc.older_than_this = &oldest_jif; - oldest_jif = jiffies - -msecs_to_jiffies (dirty_expire_interval *10 ); - }if (!wbc.range_cyclic) { wbc.range_start =0 ; wbc.range_end =LLONG_MAX ;@@ -998,9 +995,6 @@ EXPORT_SYMBOL(__mark_inode_dirty);* Write out a superblock's list of dirty inodes. A wait will be performed * upon no inodes, all inodes or the final one, depending upon sync_mode. * -* If older_than_this is non-NULL, then only write out inodes which -* had their first dirtying at a time earlier than *older_than_this. -* * If `bdi' is non-zero then we're being asked to writeback a specific queue. * This function assumes that the blockdev superblock's inodes are backed by * a variety of queues, so all inodes are searched. For other superblocks,
include/linux/writeback.h enum writeback_sync_modes { · line 33
Quoted 8 lines
include/linux/writeback.h +0
-2@@ -28,8 +28,6 @@ enum writeback_sync_modes {*/ struct writeback_control {enum writeback_sync_modes sync_mode; -unsigned long *older_than_this;/* If !NULL, only write back inodes -older than this */ unsigned long wb_start;/* Time writeback_inodes_wb was called. This is needed to avoid extra jobs and livelock */
In addtion, We shuld remove older_than_this in btrfs and reiser4.
Signature · 8 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 3/6 writeback: kill writeback_control.more_io
In reply to Wu FengguangWhen wbc.more_io was first introduced, it indicates whether there are at least one superblock whose s_more_io contains more IO work. Now with the per-bdi writeback, it can be replaced with a simple b_more_io test.
--- fs/fs-writeback.c | 9 ++------- include/linux/writeback.h | 1 - include/trace/events/writeback.h | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-)
fs/fs-writeback.c +2 -7
@@ -507,12 +507,8 @@ static int writeback_sb_inodes(struct suiput (inode);cond_resched ();spin_lock (&inode_lock); -if (wbc->nr_to_write <=0 ) { - wbc->more_io =1 ; +if (wbc->nr_to_write <=0 )return 1 ; - } -if (!list_empty (&wb->b_more_io)) - wbc->more_io =1 ; }/* b_io is empty */ return 1 ;
@@ -622,7 +618,6 @@ static long wb_writeback(struct bdi_writif (work->for_background && !over_bground_thresh ())break ; - wbc.more_io =0 ; wbc.nr_to_write =MAX_WRITEBACK_PAGES ; wbc.pages_skipped =0 ;
@@ -644,7 +639,7 @@ static long wb_writeback(struct bdi_writ/* * Didn't write everything and we don't have more IO, bail */ -if (!wbc.more_io) +if (list_empty (&wb->b_more_io))break ;/* * Did we write something? Try for more
include/linux/writeback.h +0 -1
@@ -49,7 +49,6 @@ struct writeback_control { unsigned for_background:1; /* A background writeback */ unsigned for_reclaim:1; /* Invoked from the page allocator */ unsigned range_cyclic:1; /* range_start is cyclic */ - unsigned more_io:1; /* more io to be dispatched */ };/*
include/trace/events/writeback.h +1 -4
@@ -99,7 +99,6 @@ DECLARE_EVENT_CLASS(wbc_class, __field(int, for_background) __field(int, for_reclaim) __field(int, range_cyclic) - __field(int, more_io) __field(long, range_start)__field (long, range_end) ),
@@ -113,13 +112,12 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_background = wbc->for_background; __entry->for_reclaim = wbc->for_reclaim; __entry->range_cyclic = wbc->range_cyclic; - __entry->more_io = wbc->more_io; __entry->range_start = (long )wbc->range_start; __entry->range_end = (long )wbc->range_end; ), TP_printk("bdi %s: towrt=%ld skip=%ld mode=%d kupd=%d " - "bgrd=%d reclm=%d cyclic=%d more=%d " + "bgrd=%d reclm=%d cyclic=%d " "start=0x%lx end=0x%lx", __entry->name, __entry->nr_to_write,
@@ -129,7 +127,6 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_background, __entry->for_reclaim, __entry->range_cyclic, - __entry->more_io, __entry->range_start, __entry->range_end) )
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/6] writeback: kill writeback_control.more_io
Quoted 3 lines · Wu Fengguang
When wbc.more_io was first introduced, it indicates whether there are at least one superblock whose
s_more_iocontains more IO work. Now with the per-bdi writeback, it can be replaced with a simpleb_more_iotest.
Looks fine.
Quoted 90 lines
Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 9 ++------- include/linux/writeback.h | 1 - include/trace/events/writeback.h | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-)fs/fs-writeback.c +2
-7@@ -507,12 +507,8 @@ static int writeback_sb_inodes(struct suiput (inode);cond_resched ();spin_lock (&inode_lock); -if (wbc->nr_to_write <=0 ) { - wbc->more_io =1 ; +if (wbc->nr_to_write <=0 )return 1 ; - } -if (!list_empty (&wb->b_more_io)) - wbc->more_io =1 ; }/* b_io is empty */ return 1 ;@@ -622,7 +618,6 @@ static long wb_writeback(struct bdi_writif (work->for_background && !over_bground_thresh ())break ; - wbc.more_io =0 ; wbc.nr_to_write =MAX_WRITEBACK_PAGES ; wbc.pages_skipped =0 ;@@ -644,7 +639,7 @@ static long wb_writeback(struct bdi_writ/* * Didn't write everything and we don't have more IO, bail */ -if (!wbc.more_io) +if (list_empty (&wb->b_more_io))break ;/* * Did we write something? Try for more include/linux/writeback.h +0
-1@@ -49,7 +49,6 @@ struct writeback_control { unsigned for_background:1; /* A background writeback */ unsigned for_reclaim:1; /* Invoked from the page allocator */ unsigned range_cyclic:1; /* range_start is cyclic */ - unsigned more_io:1; /* more io to be dispatched */ };/* include/trace/events/writeback.h +1
-5@@ -99,7 +99,6 @@ DECLARE_EVENT_CLASS(wbc_class, __field(int, for_background) __field(int, for_reclaim) __field(int, range_cyclic) - __field(int, more_io) __field(long, range_start)__field (long, range_end) ),@@ -113,13 +112,12 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_background = wbc->for_background; __entry->for_reclaim = wbc->for_reclaim; __entry->range_cyclic = wbc->range_cyclic; - __entry->more_io = wbc->more_io; __entry->range_start = (long )wbc->range_start; __entry->range_end = (long )wbc->range_end; ), TP_printk("bdi %s: towrt=%ld skip=%ld mode=%d kupd=%d " - "bgrd=%d reclm=%d cyclic=%d more=%d " + "bgrd=%d reclm=%d cyclic=%d " "start=0x%lx end=0x%lx", __entry->name, __entry->nr_to_write,@@ -129,7 +127,6 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_background, __entry->for_reclaim, __entry->range_cyclic, - __entry->more_io, __entry->range_start, __entry->range_end) ) --To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/6] writeback: kill writeback_control.more_io
In reply to Wu FengguangQuoted 5 lines · Wu Fengguang
When wbc.more_io was first introduced, it indicates whether there are at least one superblock whose
s_more_iocontains more IO work. Now with the per-bdi writeback, it can be replaced with a simpleb_more_iotest.Signed-off-by Wu Fengguang <[email protected]>
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/6] writeback: kill writeback_control.more_io
In reply to Wu FengguangQuoted 80 lines · Wu Fengguang
When wbc.more_io was first introduced, it indicates whether there are at least one superblock whose
s_more_iocontains more IO work. Now with the per-bdi writeback, it can be replaced with a simpleb_more_iotest.Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 9 ++------- include/linux/writeback.h | 1 - include/trace/events/writeback.h | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-)fs/fs-writeback.c +2
-7@@ -507,12 +507,8 @@ static int writeback_sb_inodes(struct suiput (inode);cond_resched ();spin_lock (&inode_lock); -if (wbc->nr_to_write <=0 ) { - wbc->more_io =1 ; +if (wbc->nr_to_write <=0 )return 1 ; - } -if (!list_empty (&wb->b_more_io)) - wbc->more_io =1 ; }/* b_io is empty */ return 1 ;@@ -622,7 +618,6 @@ static long wb_writeback(struct bdi_writif (work->for_background && !over_bground_thresh ())break ; - wbc.more_io =0 ; wbc.nr_to_write =MAX_WRITEBACK_PAGES ; wbc.pages_skipped =0 ;@@ -644,7 +639,7 @@ static long wb_writeback(struct bdi_writ/* * Didn't write everything and we don't have more IO, bail */ -if (!wbc.more_io) +if (list_empty (&wb->b_more_io))break ;/* * Did we write something? Try for more include/linux/writeback.h +0
-1@@ -49,7 +49,6 @@ struct writeback_control { unsigned for_background:1; /* A background writeback */ unsigned for_reclaim:1; /* Invoked from the page allocator */ unsigned range_cyclic:1; /* range_start is cyclic */ - unsigned more_io:1; /* more io to be dispatched */ };/* include/trace/events/writeback.h +1
-3@@ -99,7 +99,6 @@ DECLARE_EVENT_CLASS(wbc_class, __field(int, for_background) __field(int, for_reclaim) __field(int, range_cyclic) - __field(int, more_io) __field(long, range_start)__field (long, range_end) ),@@ -113,13 +112,12 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_background = wbc->for_background; __entry->for_reclaim = wbc->for_reclaim; __entry->range_cyclic = wbc->range_cyclic; - __entry->more_io = wbc->more_io; __entry->range_start = (long )wbc->range_start; __entry->range_end = (long )wbc->range_end; ), TP_printk("bdi %s: towrt=%ld skip=%ld mode=%d kupd=%d " - "bgrd=%d reclm=%d cyclic=%d more=%d " + "bgrd=%d reclm=%d cyclic=%d " "start=0x%lx end=0x%lx", __entry->name, __entry->nr_to_write,
include/trace/events/writeback.h DECLARE_EVENT_CLASS(wbc_class,
Quoted 10 lines
include/trace/events/writeback.h +0
-2@@ -129,7 +127,6 @@ DECLARE_EVENT_CLASS(wbc_class, __entry->for_background, __entry->for_reclaim, __entry->range_cyclic, - __entry->more_io, __entry->range_start, __entry->range_end) ) --
include/trace/events/ext4.h also have more_io field.
Signature · 8 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/6] writeback: kill writeback_control.more_io
Quoted 1 line
include/trace/events/ext4.halso havemore_iofield.
I didn't find it in linux-next. What's your kernel version?
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/6] writeback: kill writeback_control.more_io
Quoted 3 lines · Wu Fengguang
Quoted 1 line
include/trace/events/ext4.halso havemore_iofield.I didn't find it in linux-next. What's your kernel version?
Oh it's in mmotm :)
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/6] writeback: kill writeback_control.more_io
In reply to Wu FengguangQuoted 3 lines · Wu Fengguang
Quoted 1 line
include/trace/events/ext4.halso havemore_iofield.I didn't find it in linux-next. What's your kernel version?
I used mmotm-07-29.
Quoted 3 lines
Thanks,
Fengguang
Signature · 8 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/6] writeback: kill writeback_control.more_io
Quoted 6 lines · Minchan Kim
Quoted 3 lines · Wu Fengguang
Quoted 1 line
include/trace/events/ext4.halso havemore_iofield.I didn't find it in linux-next. What's your kernel version?
I used mmotm-07-29.
Heh it's in linux-next too -- I didn't find the field because the chunk to remove it slipped into a previous patch..
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 4/6 writeback: sync expired inodes first in background writeback
In reply to Wu FengguangA 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
--- fs/fs-writeback.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
fs/fs-writeback.c +14 -6
@@ -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 lwhile (!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 ; + }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);
Jan Kara
maintainerRe: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 6 lines · Wu Fengguang
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 eachqueue_io()time
- retry with halfed expire interval until get some inodes to sync
Hmm, this logic looks a bit arbitrary to me. What I actually don't like
very much about this that when there aren't inodes older than say 2 seconds, you'll end up queueing just inodes between 2s and 1s. So I'd rather just queue inodes older than the limit and if there are none, just queue all other dirty inodes.
Honza
Quoted 68 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)fs/fs-writeback.c +14
-7@@ -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 lwhile (!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 ; + }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); --To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 4 lines · Jan Kara
Quoted 3 lines · Wu Fengguang
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.
Quoted 8 lines
Quoted 3 lines
The policy is
- enqueue all newly expired inodes at eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncHmm, this logic looks a bit arbitrary to me. What I actually don't like
very much about this that when there aren't inodes older than say 2 seconds, you'll end up queueing just inodes between 2s and 1s. So I'd rather just queue inodes older than the limit and if there are none, just queue all other dirty inodes.
You are proposing
- expire_interval >>= 1;
+ expire_interval = 0;
IMO this does not really simplify code or concept. If we can get the "smoother" behavior in original patch without extra cost, why not?
Thanks,
Fengguang
Quoted 64 lines
Quoted 64 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)fs/fs-writeback.c +14
-6@@ -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 lwhile (!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 ; + }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);
Quoted 7 lines
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.htmlSignature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Jan Kara
maintainerRe: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 19 lines · Wu Fengguang
Quoted 4 lines · Jan Kara
Quoted 3 lines · Wu Fengguang
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.
Quoted 8 lines
Quoted 3 lines
The policy is
- enqueue all newly expired inodes at eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncHmm, this logic looks a bit arbitrary to me. What I actually don't like
very much about this that when there aren't inodes older than say 2 seconds, you'll end up queueing just inodes between 2s and 1s. So I'd rather just queue inodes older than the limit and if there are none, just queue all other dirty inodes.You are proposing
(quoted diff) +1
-1- expire_interval >>= 1; + expire_interval = 0;
Quoted 2 lines
IMO this does not really simplify code or concept. If we can get the "smoother" behavior in original patch without extra cost, why not?
I agree there's no substantial code simplification. But I see a
substantial "behavior" simplification (just two sweeps instead of 10 or so). But I don't really insist on the two sweeps, it's just that I don't see a justification for the exponencial back off here... I mean what's the point if the interval we queue gets really small? Why not just use expire_interval/2 as a step if you want a smoother behavior?
Honza
Quoted 71 lines
Quoted 71 lines
Quoted 68 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)fs/fs-writeback.c +14
-7@@ -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 lwhile (!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 ; + }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); --To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.htmlSignature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 20 lines · Jan Kara
Quoted 19 lines · Wu Fengguang
Quoted 4 lines · Jan Kara
Quoted 3 lines · Wu Fengguang
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.
Quoted 8 lines
Quoted 3 lines
The policy is
- enqueue all newly expired inodes at eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncHmm, this logic looks a bit arbitrary to me. What I actually don't like
very much about this that when there aren't inodes older than say 2 seconds, you'll end up queueing just inodes between 2s and 1s. So I'd rather just queue inodes older than the limit and if there are none, just queue all other dirty inodes.You are proposing
(quoted diff) +1
-1- expire_interval >>= 1; + expire_interval = 0;
Quoted 8 lines
Quoted 2 lines
IMO this does not really simplify code or concept. If we can get the "smoother" behavior in original patch without extra cost, why not?
I agree there's no substantial code simplification. But I see a
substantial "behavior" simplification (just two sweeps instead of 10 or so). But I don't really insist on the two sweeps, it's just that I don't see a justification for the exponencial back off here... I mean what's the point if the interval we queue gets really small? Why not just use expire_interval/2 as a step if you want a smoother behavior?
Yeah, the _non-linear_ backoff is not good. You have a point about the behavior simplification, and it does remove one line. So I'll follow your way.
Thanks,
Fengguang
---
Subject: writeback: sync expired inodes first in background writeback
From: Wu Fengguang <[email protected]>
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.
--- fs/fs-writeback.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
fs/fs-writeback.c +13 -6
@@ -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,14 @@ static void move_expired_inodes(struct lwhile (!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 =0 ; +continue ; + }else +break ; + }if (sb && sb != inode->i_sb) do_sb_sort =1 ; sb = inode->i_sb;
@@ -521,7 +527,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 +557,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);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
In reply to Wu FengguangQuoted 9 lines · Wu Fengguang
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 eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncCC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>
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.
Quoted 28 lines
--- fs/fs-writeback.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)fs/fs-writeback.c +2
-3@@ -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 lwhile (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); if (expire_interval && -inode_dirtied_after (inode, older_than_this))
fs/fs-writeback.c static void move_expired_inodes(struct l · line 243
Quoted 10 lines
fs/fs-writeback.c +9
-1-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.
Quoted 24 lines
fs/fs-writeback.c +3
-2if (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);
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 8 lines · Mel Gorman
Quoted 7 lines · Wu Fengguang
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 eachqueue_io()time
- retry with halfed expire interval until get some inodes to sync
Quoted 7 lines
Quoted 2 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>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.
Good suggestion. I'll add these lines:
This is to 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.
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Jan Kara
maintainerRe: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 17 lines · Wu Fengguang
Quoted 8 lines · Mel Gorman
Quoted 7 lines · Wu Fengguang
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 eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncQuoted 7 lines
Quoted 2 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>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.
Quoted 5 lines
Good suggestion. I'll add these lines:
This is to 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
Well, this kind of implicitely assumes that once page is written, it
doesn't get accessed anymore, right? Which I imagine is often true but not for all workloads... Anyway I think this behavior is a good start also because it is kind of natural to users to see "old" files written first.
Quoted 2 lines
syncing older inodes first helps reducing the dirty pages reached by the page reclaim code.
Honza
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 18 lines · Jan Kara
Quoted 17 lines · Wu Fengguang
Quoted 15 lines · Mel Gorman
Quoted 9 lines · Wu Fengguang
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 eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncCC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>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.
Quoted 7 lines
Quoted 5 lines
Good suggestion. I'll add these lines:
This is to 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. SoWell, this kind of implicitely assumes that once page is written, it
doesn't get accessed anymore, right?
No, this patch is not evicting the page :)
Quoted 4 lines
Which I imagine is often true but
not for all workloads... Anyway I think this behavior is a good start also because it is kind of natural to users to see "old" files written first.
Thanks,
Fengguang
Quoted 7 lines
Quoted 2 lines
syncing older inodes first helps reducing the dirty pages reached by the page reclaim code.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Jan Kara
maintainerRe: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 21 lines · Wu Fengguang
Quoted 20 lines · Jan Kara
Quoted 19 lines · Wu Fengguang
Quoted 8 lines · Mel Gorman
Quoted 7 lines · Wu Fengguang
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 eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncQuoted 7 lines
Quoted 2 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>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.
Good suggestion. I'll add these lines:
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
This is to 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. SoWell, this kind of implicitely assumes that once page is written, it
doesn't get accessed anymore, right?No, this patch is not evicting the page :)
Sorry, I probably wasn't clear enough :) I meant: The claim that "older
inodes contain older dirty pages, which are more close to the end of the LRU lists" assumes that once page is written it doesn't get accessed again. For example files which get continual random access (like DB files) can have rather old dirtied_when but some of their pages are accessed quite often...
Quoted 4 lines
Quoted 4 lines
Which I imagine is often true but
not for all workloads... Anyway I think this behavior is a good start also because it is kind of natural to users to see "old" files written first.
Honza
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 28 lines · Jan Kara
Quoted 27 lines · Wu Fengguang
Quoted 25 lines · Jan Kara
Quoted 17 lines · Wu Fengguang
Quoted 8 lines · Mel Gorman
Quoted 7 lines · Wu Fengguang
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 eachqueue_io()time
- retry with halfed expire interval until get some inodes to syncQuoted 7 lines
Quoted 2 lines
CC Jan Kara <[email protected]>Signed-off-by Wu Fengguang <[email protected]>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.
Quoted 5 lines
Good suggestion. I'll add these lines:
This is to 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. SoWell, this kind of implicitely assumes that once page is written, it
doesn't get accessed anymore, right?
Quoted 7 lines
Quoted 1 line
No, this patch is not evicting the page :)
Sorry, I probably wasn't clear enough :) I meant: The claim that "older
inodes contain older dirty pages, which are more close to the end of the LRU lists" assumes that once page is written it doesn't get accessed again. For example files which get continual random access (like DB files) can have rather olddirtied_whenbut some of their pages are accessed quite often...
Ah yes. That leads to another fact: smaller inodes tend to have more strong correlations between its inode dirty age and pages' dirty age.
This is one of the reason to not sync huge dirty inode in one shot. Instead of
sync 1G for inode A
sync 10M for inode B
sync 10M for inode C
sync 10M for inode D
It's better to
sync 128M for inode A
sync 10M for inode B
sync 10M for inode C
sync 10M for inode D
sync 128M for inode A
sync 128M for inode A
sync 128M for inode A
sync 10M for inode E (newly expired)
sync 128M for inode A
...
Thanks,
Fengguang
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
In reply to Mel GormanQuoted 18 lines
Quoted 5 lines
fs/fs-writeback.c +0
-1@@ -232,8 +232,15 @@ static void move_expired_inodes(struct lwhile (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); if (expire_interval && -inode_dirtied_after (inode, older_than_this))fs/fs-writeback.c static void move_expired_inodes(struct l · line 243
Quoted 10 lines
fs/fs-writeback.c +9
-1-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.
Quoted 4 lines
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.
+ */+ 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;Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 27 lines · Wu Fengguang
Quoted 18 lines
Quoted 5 lines
fs/fs-writeback.c +0
-1@@ -232,8 +232,15 @@ static void move_expired_inodes(struct lwhile (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); if (expire_interval && -inode_dirtied_after (inode, older_than_this))fs/fs-writeback.c static void move_expired_inodes(struct l · line 243
Quoted 10 lines
fs/fs-writeback.c +9
-1-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.
Quoted 4 lines
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?
(quoted diff) +0
-0@@ -232,8 +232,20 @@ static void move_expired_inodes(struct l while (!list_empty(delaying_queue)) {
Review static void move_expired_inodes(struct l · line 241
Quoted 10 lines
(quoted diff) +7
-1inode = 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
Quoted 11 lines
fs/fs-writeback.c +6
-0+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;
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 41 lines · Mel Gorman
Quoted 27 lines · Wu Fengguang
Quoted 18 lines
Quoted 5 lines
(quoted diff) +0
-1@@ -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))Review static void move_expired_inodes(struct l · line 243
Quoted 10 lines
(quoted diff) +9
-1- 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.
Quoted 4 lines
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?
(quoted diff) +0
-0@@ -232,8 +232,20 @@ static void move_expired_inodes(struct l while (!list_empty(delaying_queue)) {Review static void move_expired_inodes(struct l · line 241
Quoted 10 lines
(quoted diff) +7
-1inode = 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/
Quoted 1 line
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 <[email protected]>
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.
--- fs/fs-writeback.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
fs/fs-writeback.c +18 -5
@@ -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,20 @@ static void move_expired_inodes(struct lwhile (!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 reduce the +* number of dirty pages reaching the end of LRU lists +* and cause trouble to the page reclaim. +*/ +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;
@@ -521,7 +533,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 +563,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);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Mel Gorman
Acked-byRe: [PATCH 4/6] writeback: sync expired inodes first in background writeback
Quoted 57 lines · Wu Fengguang
Quoted 41 lines · Mel Gorman
Quoted 27 lines · Wu Fengguang
Quoted 18 lines
Quoted 5 lines
fs/fs-writeback.c +0
-1@@ -232,8 +232,15 @@ static void move_expired_inodes(struct lwhile (!list_empty (delaying_queue)) { inode = list_entry(delaying_queue->prev, struct inode, i_list); if (expire_interval && -inode_dirtied_after (inode, older_than_this))fs/fs-writeback.c static void move_expired_inodes(struct l · line 243
Quoted 10 lines
fs/fs-writeback.c +9
-1-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.
Quoted 4 lines
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?
fs/fs-writeback.c +0
-0@@ -232,8 +232,20 @@ static void move_expired_inodes(struct lwhile (!list_empty (delaying_queue)) {fs/fs-writeback.c static void move_expired_inodes(struct l · line 241
Quoted 10 lines
fs/fs-writeback.c +7
-1inode = 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/
Quoted 1 line
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 <[email protected]>
Date: Wed Jul 21 20:11:53 CST 2010A 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 eachqueue_io()time
- enqueue all dirty inodes if there are no more expired inodes to sync
Quoted 8 lines
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 <[email protected]>Signed-off-by Wu Fengguang <[email protected]>
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback
In reply to Wu FengguangHi Wu,
Quoted 26 lines
Subject: writeback: sync expired inodes first in background writeback
From: Wu Fengguang <[email protected]>
Date: Wed Jul 21 20:11:53 CST 2010A 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 eachqueue_io()time
- enqueue all dirty inodes if there are no more expired inodes to syncThis 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 <[email protected]>Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
fs/fs-writeback.c static void move_expired_inodes(struct l · line 220
Quoted 5 lines
fs/fs-writeback.c +1
-1@@ -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 */
Maybe I am rather late.
Nitpick.uninitialized_var is consistent. :)
I haven't followed up this patch series. but his patch series is a fundamental way to go for reducing pageout.
Signature · 8 lines
Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 5/6 writeback: try more writeback as long as something was written
In reply to Wu Fengguangwriteback_inodes_wb()/__writeback_inodes_sb() are not agressive in that they only populate b_io when necessary at entrance time. When the queued set of inodes are all synced, they just return, possibly with wbc.nr_to_write > 0.
For kupdate and background writeback, there may be more eligible inodes sitting in b_dirty when the current set of b_io inodes are completed. So it is necessary to try another round of writeback as long as we made some progress in this round. When there are no more eligible inodes, no more inodes will be enqueued in queue_io(), hence nothing could/will be synced and we may safely bail.
This will livelock sync when there are heavy dirtiers. However in that case sync will already be livelocked w/o this patch, as the current livelock avoidance code is virtually a no-op (for one thing, wb_time should be set statically at sync start time and be used in move_expired_inodes()). The sync livelock problem will be addressed in other patches.
--- fs/fs-writeback.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
fs/fs-writeback.c +11 -8
@@ -640,20 +640,23 @@ static long wb_writeback(struct bdi_writ wrote +=MAX_WRITEBACK_PAGES - wbc.nr_to_write;/* -* If we consumed everything, see if we have more +* Did we write something? Try for more +* +* This is needed _before_ the b_more_io test because the +* background writeback moves inodes to b_io and works on +* them in batches (in order to sync old pages first). The +* completion of the current batch does not necessarily mean +* the overall work is done. */ -if (wbc.nr_to_write <=0 ) +if (wbc.nr_to_write <MAX_WRITEBACK_PAGES )continue ; +/* -* Didn't write everything and we don't have more IO, bail +* Nothing written and no more inodes for IO, bail */ if (list_empty (&wb->b_more_io))break ; -/* -* Did we write something? Try for more -*/ -if (wbc.nr_to_write <MAX_WRITEBACK_PAGES ) -continue ; +/* * Nothing written. Wait for some inode to * become available for writeback. Otherwise
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Jan Kara
maintainerRe: [PATCH 5/6] writeback: try more writeback as long as something was written
Quoted 12 lines · Wu Fengguang
writeback_inodes_wb()/__writeback_inodes_sb() are not agressive in that they only populate
b_iowhen necessary at entrance time. When the queued set of inodes are all synced, they just return, possibly with wbc.nr_to_write > 0.For kupdate and background writeback, there may be more eligible inodes sitting in
b_dirtywhen the current set ofb_ioinodes are completed. So it is necessary to try another round of writeback as long as we made some progress in this round. When there are no more eligible inodes, no more inodes will be enqueued inqueue_io(), hence nothing could/will be synced and we may safely bail.
Quoted 5 lines
This will livelock sync when there are heavy dirtiers. However in that case sync will already be livelocked w/o this patch, as the current livelock avoidance code is virtually a no-op (for one thing,
wb_timeshould be set statically at sync start time and be used inmove_expired_inodes()). The sync livelock problem will be addressed in other patches.
Hmm, any reason why you don't solve this problem by just removing the
condition before queue_io()? It would also make the logic simpler - always queue all inodes that are eligible for writeback...
Honza
Quoted 45 lines
Signed-off-by Wu Fengguang <[email protected]>--- fs/fs-writeback.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)fs/fs-writeback.c +11
-9@@ -640,20 +640,23 @@ static long wb_writeback(struct bdi_writ wrote +=MAX_WRITEBACK_PAGES - wbc.nr_to_write;/* -* If we consumed everything, see if we have more +* Did we write something? Try for more +* +* This is needed _before_ the b_more_io test because the +* background writeback moves inodes to b_io and works on +* them in batches (in order to sync old pages first). The +* completion of the current batch does not necessarily mean +* the overall work is done. */ -if (wbc.nr_to_write <=0 ) +if (wbc.nr_to_write <MAX_WRITEBACK_PAGES )continue ; +/* -* Didn't write everything and we don't have more IO, bail +* Nothing written and no more inodes for IO, bail */ if (list_empty (&wb->b_more_io))break ; -/* -* Did we write something? Try for more -*/ -if (wbc.nr_to_write <MAX_WRITEBACK_PAGES ) -continue ; +/* * Nothing written. Wait for some inode to * become available for writeback. Otherwise -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 8 lines
Jan Kara <[email protected]> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/6] writeback: try more writeback as long as something was written
Quoted 13 lines · Jan Kara
Quoted 12 lines · Wu Fengguang
writeback_inodes_wb()/__writeback_inodes_sb() are not agressive in that they only populate
b_iowhen necessary at entrance time. When the queued set of inodes are all synced, they just return, possibly with wbc.nr_to_write > 0.For kupdate and background writeback, there may be more eligible inodes sitting in
b_dirtywhen the current set ofb_ioinodes are completed. So it is necessary to try another round of writeback as long as we made some progress in this round. When there are no more eligible inodes, no more inodes will be enqueued inqueue_io(), hence nothing could/will be synced and we may safely bail.
Quoted 7 lines
Quoted 5 lines
This will livelock sync when there are heavy dirtiers. However in that case sync will already be livelocked w/o this patch, as the current livelock avoidance code is virtually a no-op (for one thing,
wb_timeshould be set statically at sync start time and be used inmove_expired_inodes()). The sync livelock problem will be addressed in other patches.Hmm, any reason why you don't solve this problem by just removing the
condition beforequeue_io()? It would also make the logic simpler - always
Yeah I'll remove queue_io() in the coming sync livelock patchset. This patchset does the below. Though awkward, it avoids unnecessary behavior changes for non-background cases.
- 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);Quoted 1 line
queue all inodes that are eligible for writeback...
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/6] writeback: try more writeback as long as something was written
In reply to Wu FengguangQuoted 12 lines · Wu Fengguang
writeback_inodes_wb()/__writeback_inodes_sb() are not agressive in that they only populate
b_iowhen necessary at entrance time. When the queued set of inodes are all synced, they just return, possibly with wbc.nr_to_write > 0.For kupdate and background writeback, there may be more eligible inodes sitting in
b_dirtywhen the current set ofb_ioinodes are completed. So it is necessary to try another round of writeback as long as we made some progress in this round. When there are no more eligible inodes, no more inodes will be enqueued inqueue_io(), hence nothing could/will be synced and we may safely bail.
Quoted 6 lines
This will livelock sync when there are heavy dirtiers. However in that case sync will already be livelocked w/o this patch, as the current livelock avoidance code is virtually a no-op (for one thing,
wb_timeshould be set statically at sync start time and be used inmove_expired_inodes()). The sync livelock problem will be addressed in other patches.
There does seem to be a livelock issue. During iozone, I see messages in the console log with this series applied that look like
[ 1687.132034] INFO: task iozone:21225 blocked for more than 120 seconds. [ 1687.211425] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 1687.305204] iozone D ffff880001b13640 0 21225 21108 0x00000000 [ 1687.387677] ffff880037419d48 0000000000000082 0000000000000348 0000000000013640 [ 1687.476594] ffff880037419fd8 ffff880037419fd8 ffff880065892da0 0000000000013640 [ 1687.565512] 0000000000013640 0000000000013640 ffff880065892da0 ffff88007f411510 [ 1687.654431] Call Trace: [ 1687.683663] [<ffffffff81002996>] ? ftrace_call+0x5/0x2b [ 1687.747204] [<ffffffff812d8f67>] schedule_timeout+0x2d/0x214 [ 1687.815947] [<ffffffff81002996>] ? ftrace_call+0x5/0x2b [ 1687.879489] [<ffffffff812d8527>] wait_for_common+0xd2/0x14a [ 1687.947195] [<ffffffff8103ef1e>] ? default_wake_function+0x0/0x14 [ 1688.021132] [<ffffffff81002996>] ? ftrace_call+0x5/0x2b [ 1688.084680] [<ffffffff811160f0>] ? sync_one_sb+0x0/0x22 [ 1688.148223] [<ffffffff812d8657>] wait_for_completion+0x1d/0x1f [ 1688.219051] [<ffffffff811121c4>] sync_inodes_sb+0x92/0x14c [ 1688.285710] [<ffffffff811160f0>] ? sync_one_sb+0x0/0x22 [ 1688.349249] [<ffffffff811160b9>] __sync_filesystem+0x4c/0x83 [ 1688.417995] [<ffffffff81116110>] sync_one_sb+0x20/0x22 [ 1688.480505] [<ffffffff810f6a23>] iterate_supers+0x66/0xa4 [ 1688.546124] [<ffffffff81116157>] sys_sync+0x45/0x5c [ 1688.605509] [<ffffffff81002c72>] system_call_fastpath+0x16/0x1b
Similar messages do not appear without the patch. iozone does complete though and the performance figures are not affected. Should I be worried?
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/6] writeback: try more writeback as long as something was written
Quoted 46 lines · Mel Gorman
Quoted 12 lines · Wu Fengguang
writeback_inodes_wb()/__writeback_inodes_sb() are not agressive in that they only populate
b_iowhen necessary at entrance time. When the queued set of inodes are all synced, they just return, possibly with wbc.nr_to_write > 0.For kupdate and background writeback, there may be more eligible inodes sitting in
b_dirtywhen the current set ofb_ioinodes are completed. So it is necessary to try another round of writeback as long as we made some progress in this round. When there are no more eligible inodes, no more inodes will be enqueued inqueue_io(), hence nothing could/will be synced and we may safely bail.Quoted 6 lines
This will livelock sync when there are heavy dirtiers. However in that case sync will already be livelocked w/o this patch, as the current livelock avoidance code is virtually a no-op (for one thing,
wb_timeshould be set statically at sync start time and be used inmove_expired_inodes()). The sync livelock problem will be addressed in other patches.There does seem to be a livelock issue. During iozone, I see messages in the console log with this series applied that look like
[ 1687.132034] INFO: task iozone:21225 blocked for more than 120 seconds. [ 1687.211425] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 1687.305204] iozone D ffff880001b13640 0 21225 21108 0x00000000 [ 1687.387677] ffff880037419d48 0000000000000082 0000000000000348 0000000000013640 [ 1687.476594] ffff880037419fd8 ffff880037419fd8 ffff880065892da0 0000000000013640 [ 1687.565512] 0000000000013640 0000000000013640 ffff880065892da0 ffff88007f411510 [ 1687.654431] Call Trace: [ 1687.683663] [<ffffffff81002996>] ? ftrace_call+0x5/0x2b [ 1687.747204] [<ffffffff812d8f67>] schedule_timeout+0x2d/0x214 [ 1687.815947] [<ffffffff81002996>] ? ftrace_call+0x5/0x2b [ 1687.879489] [<ffffffff812d8527>] wait_for_common+0xd2/0x14a [ 1687.947195] [<ffffffff8103ef1e>] ? default_wake_function+0x0/0x14 [ 1688.021132] [<ffffffff81002996>] ? ftrace_call+0x5/0x2b [ 1688.084680] [<ffffffff811160f0>] ? sync_one_sb+0x0/0x22 [ 1688.148223] [<ffffffff812d8657>] wait_for_completion+0x1d/0x1f [ 1688.219051] [<ffffffff811121c4>] sync_inodes_sb+0x92/0x14c [ 1688.285710] [<ffffffff811160f0>] ? sync_one_sb+0x0/0x22 [ 1688.349249] [<ffffffff811160b9>] __sync_filesystem+0x4c/0x83 [ 1688.417995] [<ffffffff81116110>] sync_one_sb+0x20/0x22 [ 1688.480505] [<ffffffff810f6a23>] iterate_supers+0x66/0xa4 [ 1688.546124] [<ffffffff81116157>] sys_sync+0x45/0x5c [ 1688.605509] [<ffffffff81002c72>] system_call_fastpath+0x16/0x1b
Quoted 2 lines
Similar messages do not appear without the patch. iozone does complete though and the performance figures are not affected. Should I be worried?
The patch does add a bit more livelock possibility. But don't worry, I'll fix that.
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 6/6 writeback: introduce writeback_control.inodes_written
In reply to Wu FengguangIntroduce writeback_control.inodes_written to count successful ->write_inode() calls. A non-zero value means there are some progress on writeback, in which case more writeback will be tried.
This prevents aborting a background writeback work prematually when the current set of inodes for IO happen to be metadata-only dirty.
--- fs/fs-writeback.c | 5 +++++ include/linux/writeback.h | 1 + 2 files changed, 6 insertions(+)
fs/fs-writeback.c +5 -0
@@ -379,6 +379,8 @@ writeback_single_inode(struct inode *inoint err =write_inode (inode, wbc);if (ret ==0 ) ret = err; +if (!err) + wbc->inodes_written++; }spin_lock (&inode_lock);
@@ -628,6 +630,7 @@ static long wb_writeback(struct bdi_writ wbc.nr_to_write =MAX_WRITEBACK_PAGES ; wbc.pages_skipped =0 ; + wbc.inodes_written =0 ;trace_wbc_writeback_start (&wbc, wb->bdi);if (work->sb)
@@ -650,6 +653,8 @@ static long wb_writeback(struct bdi_writ*/ if (wbc.nr_to_write <MAX_WRITEBACK_PAGES )continue ; +if (wbc.inodes_written) +continue ;/* * Nothing written and no more inodes for IO, bail
include/linux/writeback.h +1 -0
@@ -34,6 +34,7 @@ struct writeback_control {long nr_to_write;/* Write this many pages, and decrement this for each page written */ long pages_skipped;/* Pages which were not written */ +long inodes_written;/* Number of inodes(metadata) synced */ /* * For a_ops->writepages(): is start or end are non-zero then this is
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 6/6] writeback: introduce writeback_control.inodes_written
Quoted 8 lines · Wu Fengguang
Introduce writeback_control.inodes_written to count successful ->write_inode() calls. A non-zero value means there are some progress on writeback, in which case more writeback will be tried.
This prevents aborting a background writeback work prematually when the current set of inodes for IO happen to be metadata-only dirty.
Signed-off-by Wu Fengguang <[email protected]>
Seems reasonable.
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
I queued these up for testing yesterday before starting a review. For anyone watching, the following patches are pre-requisites from linux-next if one wants to test against 2.6.35-rc5. I did this because I wanted to test as few changes as possible
a75db72d30a6402f4b1d841af3b4ce43682d0ac4 writeback: remove wb_list 2225753c10aef6af9c764a295b71d11bc483c4d6 writeback: merge bdi_writeback_task and bdi_start_fn aab24fcf6f5ccf0e8de3cc333559bddf9a46f11e writeback: Initial tracing support f689fba23f3819e3e0bc237c104f2ec25decc219 writeback: Add tracing to balance_dirty_pages ca43586868b49eb5a07d895708e4d257e2df814e simplify checks for I_CLEAR/I_FREEING
I applied your series on top of this and fired it up. The ordering of patch application was still teh same
tracing
no direct writeback
Wu's patches and Christoph's pre-reqs from linux-next Kick flusher threads when dirty pages applied
With them applied, btrfs failed to build but if it builds for you, it just means I didn't bring a required patch from linux-next. I was testing against XFS so I didn't dig too deep.
Quoted 9 lines · Wu Fengguang
The basic way of avoiding
pageout()is to make the flusher sync inodes in the right order. Oldest dirty inodes contains oldest pages. The smaller inode it is, the more correlation between inode dirty time and its pages' dirty time. So for small dirty inodes, syncing in the order of inode dirty time is able to avoidpageout(). Ifpageout()is still triggered frequently in this case, the 30s dirty expire time may be too long and could be shrinked adaptively; or it may be a stressed memcg list whose dirty inodes/pages are more hard to track.
Have you confirmed this theory with the trace points? It makes perfect sense and is very rational but proof is a plus. I'm guessing you have some decent writeback-related tests that might be of use. Mine have a big mix of anon and file writeback so it's not as clear-cut.
Monitoring it isn't hard. Mount debugfs, enable the vmscan tracepoints and read the tracing_pipe. To reduce interference, I always pipe it through gzip and do post-processing afterwards offline with the script included in Documentation/
Here is what I got from sysbench on x86-64 (other machines hours away)
SYSBENCH FTrace Reclaim Statistics
traceonly-v5r6 nodirect-v5r7 flusholdest-v5r7 flushforward-v5r7
Direct reclaims 683 785 670 938
Direct reclaim pages scanned 199776 161195 200400 166639
Direct reclaim write file async I/O 64802 0 0 0
Direct reclaim write anon async I/O 1009 419 1184 11390
Direct reclaim write file sync I/O 18 0 0 0 Direct reclaim write anon sync I/O 0 0 0 0 Wake kswapd requests 685360 697255 691009 864602 Kswapd wakeups 1596 1517 1517 1545 Kswapd pages scanned 17527865 16817554 16816510 15032525 Kswapd reclaim write file async I/O 888082 618123 649167 147903 Kswapd reclaim write anon async I/O 229724 229123 233639 243561 Kswapd reclaim write file sync I/O 0 0 0 0 Kswapd reclaim write anon sync I/O 0 0 0 0
Time stalled direct reclaim (ms) 32.79 22.47 19.75 6.34
Time kswapd awake (ms) 2192.03 2165.17 2112.73 2055.90
User/Sys Time Running Test (seconds) 663.3 656.37 664.14 654.63
Percentage Time Spent Direct Reclaim 0.00% 0.00% 0.00% 0.00%
Total Elapsed Time (seconds) 6703.22 6468.78 6472.69 6479.62
Percentage Time kswapd Awake 0.03% 0.00% 0.00% 0.00%
Flush oldest actually increased the number of pages written back by kswapd but the anon writeback is also high as swap is involved. Kicking flusher threads also helps a lot. It helps less than previous released because I noticed I was kicking flusher threads for both anon and file dirty pages which is cheating. It's now only waking the threads for file. It's still a reduction of 84% overall so nothing to sneeze at.
What the patch did do was reduce time stalled in direct reclaim and time kswapd spent awake so it still might be going the right direction. I don't have a feeling for how much the writeback figures change between runs because they take so long to run.
STRESS-HIGHALLOC FTrace Reclaim Statistics
stress-highalloc stress-highalloc stress-highalloc stress-highalloc
traceonly-v5r6 nodirect-v5r7 flusholdest-v5r7 flushforward-v5r7
Direct reclaims 1221 1284 1127 1252
Direct reclaim pages scanned 146220 186156 142075 140617
Direct reclaim write file async I/O 3433 0 0 0
Direct reclaim write anon async I/O 25238 28758 23940 23247
Direct reclaim write file sync I/O 3095 0 0 0
Direct reclaim write anon sync I/O 10911 305579 281824 246251
Wake kswapd requests 1193 1196 1088 1209
Kswapd wakeups 805 824 758 804
Kswapd pages scanned 30953364 52621368 42722498 30945547
Kswapd reclaim write file async I/O 898087 241135 570467 54319
Kswapd reclaim write anon async I/O 2278607 2201894 1885741 1949170
Kswapd reclaim write file sync I/O 0 0 0 0 Kswapd reclaim write anon sync I/O 0 0 0 0 Time stalled direct reclaim (ms) 8567.29 6628.83 6520.39 6947.23 Time kswapd awake (ms) 5847.60 3589.43 3900.74 15837.59
User/Sys Time Running Test (seconds) 2824.76 2833.05 2833.26 2830.46
Percentage Time Spent Direct Reclaim 0.25% 0.00% 0.00% 0.00%
Total Elapsed Time (seconds) 10920.14 9021.17 8872.06 9301.86
Percentage Time kswapd Awake 0.15% 0.00% 0.00% 0.00%
Same here, the number of pages written back by kswapd increased but again anon writeback was a big factor. Kicking threads when dirty pages are encountered still helps a lot with a 94% reduction of pages written back overall..
Also, your patch really helped the time spent stalled by direct reclaim and kswapd was awake a lot less less with tests completing far faster.
Overally, I still think your series if a big help (although I don't know if the patches in linux-next are also making a difference) but it's not actually reducing the pages encountered by direct reclaim. Maybe that is because the tests were making more forward progress and so scanning faster. The sysbench performance results are too varied to draw conclusions from but it did slightly improve the success rate of high-order allocations.
The flush-forward patches would appear to be a requirement. Christoph first described them as a band-aid but he didn't chuck rocks at me when the patch was actually released. Right now, I'm leaning towards pushing it and judge by the Swear Meter how good/bad others think it is. So far it's, me pro, Rik pro, Christoph maybe.
Quoted 8 lines
For a large dirty inode, it may flush lots of newly dirtied pages _after_ syncing the expired pages. This is the normal case for a single-stream sequential dirtier, where older pages are in lower offsets. In this case we shall not insist on syncing the whole large dirty inode before considering the other small dirty inodes. This risks wasting time syncing 1GB freshly dirtied pages before syncing the other N*1MB expired dirty pages who are approaching the end of the LRU list and hence
pageout().
Intuitively, this makes a lot of sense.
Quoted 5 lines
For a large dirty inode, it may also flush lots of newly dirtied pages _before_ hitting the desired old ones, in which case it helps for
pageout()to do some clustered writeback, and/or setmapping->writeback_indexto help the flusher focus on old pages.
Will put this idea on the maybe pile.
Quoted 7 lines
For a large dirty inode, it may also have intermixed old and new dirty pages. In this case we need to make sure the inode is queued for IO before some of its pages hit
pageout(). Adaptive dirty expire time helps here.OK, end of the vapour ideas. As for this patchset, it fixes the current kupdate/background writeback priority:
Quoted 7 lines
- the kupdate/background writeback shall include newly expired inodes at each
queue_io()time, as the large inodes left over from previous writeback rounds are likely to have less density of old pages.- the background writeback shall consider expired inodes first, just like the kupdate writeback
I haven't actually reviewed these. I got testing kicked off first because it didn't require brains :)
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 11 lines · Wu Fengguang
Quoted 9 lines
The basic way of avoiding
pageout()is to make the flusher sync inodes in the right order. Oldest dirty inodes contains oldest pages. The smaller inode it is, the more correlation between inode dirty time and its pages' dirty time. So for small dirty inodes, syncing in the order of inode dirty time is able to avoidpageout(). Ifpageout()is still triggered frequently in this case, the 30s dirty expire time may be too long and could be shrinked adaptively; or it may be a stressed memcg list whose dirty inodes/pages are more hard to track.
Quoted 2 lines
Have you confirmed this theory with the trace points? It makes perfect sense and is very rational but proof is a plus.
The proof would be simple.
On average, it takes longer time to dirty a large file than a small file.
For example, when uploading files to a file server with 1MB/s throughput, it will take 10s for a 10MB file and 30s for a 30MB file. This is the common case.
Another case is some fast dirtier. It may take 10ms to dirty a 100MB file and 10s to dirty a 1G file -- the latter is dirty throttled to the much lower IO throughput due to too many dirty pages. The opposite may happen, however this is more likely in possibility. If both are throttled, it degenerates to the above file server case.
So large files tend to contain dirty pages of more varied age.
Quoted 3 lines
I'm guessing you have
some decent writeback-related tests that might be of use. Mine have a big mix of anon and file writeback so it's not as clear-cut.
A neat trick is to run your test with swapoff -a :)
Seriously I have no scripts to monitor pageout() calls. I'll explore ways to test it.
Quoted 4 lines
Monitoring it isn't hard. Mount debugfs, enable the vmscan tracepoints and read the
tracing_pipe. To reduce interference, I always pipe it through gzip and do post-processing afterwards offline with the script included inDocumentation/
Thanks for the tip!
Quoted 19 lines
Here is what I got from sysbench on x86-64 (other machines hours away)
SYSBENCH FTrace Reclaim Statistics
traceonly-v5r6 nodirect-v5r7 flusholdest-v5r7 flushforward-v5r7
Direct reclaims 683 785 670 938
Direct reclaim pages scanned 199776 161195 200400 166639
Direct reclaim write file async I/O 64802 0 0 0
Direct reclaim write anon async I/O 1009 419 1184 11390Direct reclaim write file sync I/O 18 0 0 0 Direct reclaim write anon sync I/O 0 0 0 0 Wake kswapd requests 685360 697255 691009 864602 Kswapd wakeups 1596 1517 1517 1545 Kswapd pages scanned 17527865 16817554 16816510 15032525 Kswapd reclaim write file async I/O 888082 618123 649167 147903 Kswapd reclaim write anon async I/O 229724 229123 233639 243561 Kswapd reclaim write file sync I/O 0 0 0 0 Kswapd reclaim write anon sync I/O 0 0 0 0
Quoted 2 lines
Time stalled direct reclaim (ms) 32.79 22.47 19.75 6.34
Time kswapd awake (ms) 2192.03 2165.17 2112.73 2055.90
I noticed that $total_direct_latency is divided by 1000 before printing the above lines, so the unit should be seconds?
Quoted 4 lines
User/Sys Time Running Test (seconds) 663.3 656.37 664.14 654.63
Percentage Time Spent Direct Reclaim 0.00% 0.00% 0.00% 0.00%
Total Elapsed Time (seconds) 6703.22 6468.78 6472.69 6479.62
Percentage Time kswapd Awake 0.03% 0.00% 0.00% 0.00%
I don't see the code for generating the "Percentage" lines. And the numbers seem too small to be true.
Quoted 36 lines
Flush oldest actually increased the number of pages written back by kswapd but the anon writeback is also high as swap is involved. Kicking flusher threads also helps a lot. It helps less than previous released because I noticed I was kicking flusher threads for both anon and file dirty pages which is cheating. It's now only waking the threads for file. It's still a reduction of 84% overall so nothing to sneeze at.
What the patch did do was reduce time stalled in direct reclaim and time kswapd spent awake so it still might be going the right direction. I don't have a feeling for how much the writeback figures change between runs because they take so long to run.
STRESS-HIGHALLOC FTrace Reclaim Statistics
stress-highalloc stress-highalloc stress-highalloc stress-highalloc
traceonly-v5r6 nodirect-v5r7 flusholdest-v5r7 flushforward-v5r7
Direct reclaims 1221 1284 1127 1252
Direct reclaim pages scanned 146220 186156 142075 140617
Direct reclaim write file async I/O 3433 0 0 0
Direct reclaim write anon async I/O 25238 28758 23940 23247
Direct reclaim write file sync I/O 3095 0 0 0
Direct reclaim write anon sync I/O 10911 305579 281824 246251
Wake kswapd requests 1193 1196 1088 1209
Kswapd wakeups 805 824 758 804
Kswapd pages scanned 30953364 52621368 42722498 30945547
Kswapd reclaim write file async I/O 898087 241135 570467 54319
Kswapd reclaim write anon async I/O 2278607 2201894 1885741 1949170Kswapd reclaim write file sync I/O 0 0 0 0 Kswapd reclaim write anon sync I/O 0 0 0 0 Time stalled direct reclaim (ms) 8567.29 6628.83 6520.39 6947.23 Time kswapd awake (ms) 5847.60 3589.43 3900.74 15837.59User/Sys Time Running Test (seconds) 2824.76 2833.05 2833.26 2830.46
Percentage Time Spent Direct Reclaim 0.25% 0.00% 0.00% 0.00%
Total Elapsed Time (seconds) 10920.14 9021.17 8872.06 9301.86
Percentage Time kswapd Awake 0.15% 0.00% 0.00% 0.00%
Quoted 4 lines
Same here, the number of pages written back by kswapd increased but again anon writeback was a big factor. Kicking threads when dirty pages are encountered still helps a lot with a 94% reduction of pages written back overall..
That is impressive! So it definitely helps to reduce total number of dirty pages under memory pressure.
Quoted 2 lines
Also, your patch really helped the time spent stalled by direct reclaim and kswapd was awake a lot less less with tests completing far faster.
Thanks. So it does improve the dirty page layout in the LRU lists.
Quoted 7 lines
Overally, I still think your series if a big help (although I don't know if the patches in linux-next are also making a difference) but it's not actually reducing the pages encountered by direct reclaim. Maybe that is because the tests were making more forward progress and so scanning faster. The sysbench performance results are too varied to draw conclusions from but it did slightly improve the success rate of high-order allocations.
Quoted 5 lines
The flush-forward patches would appear to be a requirement. Christoph first described them as a band-aid but he didn't chuck rocks at me when the patch was actually released. Right now, I'm leaning towards pushing it and judge by the Swear Meter how good/bad others think it is. So far it's, me pro, Rik pro, Christoph maybe.
Sorry for the delay, I'll help review it.
Quoted 30 lines
Quoted 8 lines
For a large dirty inode, it may flush lots of newly dirtied pages _after_ syncing the expired pages. This is the normal case for a single-stream sequential dirtier, where older pages are in lower offsets. In this case we shall not insist on syncing the whole large dirty inode before considering the other small dirty inodes. This risks wasting time syncing 1GB freshly dirtied pages before syncing the other N*1MB expired dirty pages who are approaching the end of the LRU list and hence
pageout().Intuitively, this makes a lot of sense.
Quoted 5 lines
For a large dirty inode, it may also flush lots of newly dirtied pages _before_ hitting the desired old ones, in which case it helps for
pageout()to do some clustered writeback, and/or setmapping->writeback_indexto help the flusher focus on old pages.Will put this idea on the maybe pile.
Quoted 11 lines
For a large dirty inode, it may also have intermixed old and new dirty pages. In this case we need to make sure the inode is queued for IO before some of its pages hit
pageout(). Adaptive dirty expire time helps here.OK, end of the vapour ideas. As for this patchset, it fixes the current kupdate/background writeback priority:
- the kupdate/background writeback shall include newly expired inodes at each
queue_io()time, as the large inodes left over from previous writeback rounds are likely to have less density of old pages.
Quoted 6 lines
Quoted 3 lines
- the background writeback shall consider expired inodes first, just like the kupdate writeback
I haven't actually reviewed these. I got testing kicked off first because it didn't require brains :)
Thanks all the same!
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 22 lines · Wu Fengguang
Quoted 11 lines · Wu Fengguang
Quoted 9 lines
The basic way of avoiding
pageout()is to make the flusher sync inodes in the right order. Oldest dirty inodes contains oldest pages. The smaller inode it is, the more correlation between inode dirty time and its pages' dirty time. So for small dirty inodes, syncing in the order of inode dirty time is able to avoidpageout(). Ifpageout()is still triggered frequently in this case, the 30s dirty expire time may be too long and could be shrinked adaptively; or it may be a stressed memcg list whose dirty inodes/pages are more hard to track.Quoted 2 lines
Have you confirmed this theory with the trace points? It makes perfect sense and is very rational but proof is a plus.
The proof would be simple.
On average, it takes longer time to dirty a large file than a small file.
For example, when uploading files to a file server with 1MB/s throughput, it will take 10s for a 10MB file and 30s for a 30MB file. This is the common case.
Quoted 8 lines
Another case is some fast dirtier. It may take 10ms to dirty a 100MB file and 10s to dirty a 1G file -- the latter is dirty throttled to the much lower IO throughput due to too many dirty pages. The opposite may happen, however this is more likely in possibility. If both are throttled, it degenerates to the above file server case.
So large files tend to contain dirty pages of more varied age.
Ok.
Quoted 6 lines
Quoted 3 lines
I'm guessing you have
some decent writeback-related tests that might be of use. Mine have a big mix of anon and file writeback so it's not as clear-cut.A neat trick is to run your test with
swapoff -a:)
Good point.
Quoted 3 lines
Seriously I have no scripts to monitor
pageout()calls. I'll explore ways to test it.
I'll see about running tests with swapoff.
Quoted 26 lines
Quoted 4 lines
Monitoring it isn't hard. Mount debugfs, enable the vmscan tracepoints and read the
tracing_pipe. To reduce interference, I always pipe it through gzip and do post-processing afterwards offline with the script included inDocumentation/Thanks for the tip!
Quoted 18 lines
Here is what I got from sysbench on x86-64 (other machines hours away)
SYSBENCH FTrace Reclaim Statistics
traceonly-v5r6 nodirect-v5r7 flusholdest-v5r7 flushforward-v5r7
Direct reclaims 683 785 670 938
Direct reclaim pages scanned 199776 161195 200400 166639
Direct reclaim write file async I/O 64802 0 0 0
Direct reclaim write anon async I/O 1009 419 1184 11390Direct reclaim write file sync I/O 18 0 0 0 Direct reclaim write anon sync I/O 0 0 0 0 Wake kswapd requests 685360 697255 691009 864602 Kswapd wakeups 1596 1517 1517 1545 Kswapd pages scanned 17527865 16817554 16816510 15032525 Kswapd reclaim write file async I/O 888082 618123 649167 147903 Kswapd reclaim write anon async I/O 229724 229123 233639 243561 Kswapd reclaim write file sync I/O 0 0 0 0 Kswapd reclaim write anon sync I/O 0 0 0 0
Quoted 6 lines
Quoted 2 lines
Time stalled direct reclaim (ms) 32.79 22.47 19.75 6.34
Time kswapd awake (ms) 2192.03 2165.17 2112.73 2055.90I noticed that $total_direct_latency is divided by 1000 before printing the above lines, so the unit should be seconds?
Correct. That figure was generated by another post-processing script that creates the table. It got the units wrong so the percentage time lines are wrong and the time spent awake is saying ms when it should say seconds. Sorry about that.
Quoted 8 lines
Quoted 4 lines
User/Sys Time Running Test (seconds) 663.3 656.37 664.14 654.63
Percentage Time Spent Direct Reclaim 0.00% 0.00% 0.00% 0.00%
Total Elapsed Time (seconds) 6703.22 6468.78 6472.69 6479.62
Percentage Time kswapd Awake 0.03% 0.00% 0.00% 0.00%I don't see the code for generating the "Percentage" lines. And the numbers seem too small to be true.
The code is in a table-generation script that had access to data on the length of time the test ran. I was ignoring the percentage time line since early on and I missed the error.
The percentage time spent on direct reclaim is
direct_reclaim*100/(user_time+sys_time+stalled_time)
A report based on a corrected scipt looks like
traceonly-v5r6 nodirect-v5r9 flusholdest-v5r9 flushforward-v5r9
Direct reclaims 683 528 808 943
Direct reclaim pages scanned 199776 298562 125991 83325
Direct reclaim write file async I/O 64802 0 0 0
Direct reclaim write anon async I/O 1009 3340 926 2227
Direct reclaim write file sync I/O 18 0 0 0 Direct reclaim write anon sync I/O 0 0 0 0 Wake kswapd requests 685360 522123 763448 827895 Kswapd wakeups 1596 1538 1452 1565 Kswapd pages scanned 17527865 17020235 16367809 15415022 Kswapd reclaim write file async I/O 888082 869540 536427 89004 Kswapd reclaim write anon async I/O 229724 262934 253396 215861 Kswapd reclaim write file sync I/O 0 0 0 0 Kswapd reclaim write anon sync I/O 0 0 0 0
Time stalled direct reclaim (seconds) 32.79 23.46 20.70 7.01
Time kswapd awake (seconds) 2192.03 2172.22 2117.82 2166.53
User/Sys Time Running Test (seconds) 663.3 644.43 637.34 680.53
Percentage Time Spent Direct Reclaim 4.71% 3.51% 3.15% 1.02%
Total Elapsed Time (seconds) 6703.22 6477.95 6503.39 6781.90
Percentage Time kswapd Awake 32.70% 33.53% 32.56% 31.95%
Quoted 36 lines
Quoted 36 lines
Flush oldest actually increased the number of pages written back by kswapd but the anon writeback is also high as swap is involved. Kicking flusher threads also helps a lot. It helps less than previous released because I noticed I was kicking flusher threads for both anon and file dirty pages which is cheating. It's now only waking the threads for file. It's still a reduction of 84% overall so nothing to sneeze at.
What the patch did do was reduce time stalled in direct reclaim and time kswapd spent awake so it still might be going the right direction. I don't have a feeling for how much the writeback figures change between runs because they take so long to run.
STRESS-HIGHALLOC FTrace Reclaim Statistics
stress-highalloc stress-highalloc stress-highalloc stress-highalloc
traceonly-v5r6 nodirect-v5r7 flusholdest-v5r7 flushforward-v5r7
Direct reclaims 1221 1284 1127 1252
Direct reclaim pages scanned 146220 186156 142075 140617
Direct reclaim write file async I/O 3433 0 0 0
Direct reclaim write anon async I/O 25238 28758 23940 23247
Direct reclaim write file sync I/O 3095 0 0 0
Direct reclaim write anon sync I/O 10911 305579 281824 246251
Wake kswapd requests 1193 1196 1088 1209
Kswapd wakeups 805 824 758 804
Kswapd pages scanned 30953364 52621368 42722498 30945547
Kswapd reclaim write file async I/O 898087 241135 570467 54319
Kswapd reclaim write anon async I/O 2278607 2201894 1885741 1949170Kswapd reclaim write file sync I/O 0 0 0 0 Kswapd reclaim write anon sync I/O 0 0 0 0 Time stalled direct reclaim (ms) 8567.29 6628.83 6520.39 6947.23 Time kswapd awake (ms) 5847.60 3589.43 3900.74 15837.59User/Sys Time Running Test (seconds) 2824.76 2833.05 2833.26 2830.46
Percentage Time Spent Direct Reclaim 0.25% 0.00% 0.00% 0.00%
Total Elapsed Time (seconds) 10920.14 9021.17 8872.06 9301.86
Percentage Time kswapd Awake 0.15% 0.00% 0.00% 0.00%
Quoted 8 lines
Quoted 4 lines
Same here, the number of pages written back by kswapd increased but again anon writeback was a big factor. Kicking threads when dirty pages are encountered still helps a lot with a 94% reduction of pages written back overall..
That is impressive! So it definitely helps to reduce total number of dirty pages under memory pressure.
Yes.
Quoted 5 lines
Quoted 2 lines
Also, your patch really helped the time spent stalled by direct reclaim and kswapd was awake a lot less less with tests completing far faster.
Thanks. So it does improve the dirty page layout in the LRU lists.
It would appear to.
Quoted 7 lines
Quoted 7 lines
Overally, I still think your series if a big help (although I don't know if the patches in linux-next are also making a difference) but it's not actually reducing the pages encountered by direct reclaim. Maybe that is because the tests were making more forward progress and so scanning faster. The sysbench performance results are too varied to draw conclusions from but it did slightly improve the success rate of high-order allocations.
Quoted 8 lines
Quoted 5 lines
The flush-forward patches would appear to be a requirement. Christoph first described them as a band-aid but he didn't chuck rocks at me when the patch was actually released. Right now, I'm leaning towards pushing it and judge by the Swear Meter how good/bad others think it is. So far it's, me pro, Rik pro, Christoph maybe.
Sorry for the delay, I'll help review it.
Don't be sorry, I still haven't reviewed the writeback patches.
Quoted 39 lines
Quoted 30 lines
Quoted 8 lines
For a large dirty inode, it may flush lots of newly dirtied pages _after_ syncing the expired pages. This is the normal case for a single-stream sequential dirtier, where older pages are in lower offsets. In this case we shall not insist on syncing the whole large dirty inode before considering the other small dirty inodes. This risks wasting time syncing 1GB freshly dirtied pages before syncing the other N*1MB expired dirty pages who are approaching the end of the LRU list and hence
pageout().Intuitively, this makes a lot of sense.
Quoted 5 lines
For a large dirty inode, it may also flush lots of newly dirtied pages _before_ hitting the desired old ones, in which case it helps for
pageout()to do some clustered writeback, and/or setmapping->writeback_indexto help the flusher focus on old pages.Will put this idea on the maybe pile.
Quoted 11 lines
For a large dirty inode, it may also have intermixed old and new dirty pages. In this case we need to make sure the inode is queued for IO before some of its pages hit
pageout(). Adaptive dirty expire time helps here.OK, end of the vapour ideas. As for this patchset, it fixes the current kupdate/background writeback priority:
- the kupdate/background writeback shall include newly expired inodes at each
queue_io()time, as the large inodes left over from previous writeback rounds are likely to have less density of old pages.Quoted 6 lines
Quoted 3 lines
- the background writeback shall consider expired inodes first, just like the kupdate writeback
I haven't actually reviewed these. I got testing kicked off first because it didn't require brains :)
Thanks all the same!
Signature · 9 lines
Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Hi,
Here's a touch up patch on top of your changes against the latest mmotm.
--- fs/btrfs/extent_io.c | 2 -- include/trace/events/ext4.h | 5 +---- 2 files changed, 1 insertions(+), 6 deletions(-)
fs/btrfs/extent_io.c +0 -2
@@ -2586,7 +2586,6 @@ int extent_write_full_page(struct extent_io_tree *tree, struct page *page, };struct writeback_control wbc_writepages = { .sync_mode = wbc->sync_mode, - .older_than_this =NULL , .nr_to_write =64 , .range_start =page_offset (page) +PAGE_CACHE_SIZE , .range_end = (loff_t)-1 ,
@@ -2619,7 +2618,6 @@ int extent_write_locked_range(struct extent_io_tree *tree, struct inode *inode, };struct writeback_control wbc_writepages = { .sync_mode = mode, - .older_than_this =NULL , .nr_to_write = nr_pages *2 , .range_start = start, .range_end = end +1 ,
include/trace/events/ext4.h +1 -4
@@ -305,7 +305,6 @@ TRACE_EVENT(ext4_da_writepages_result, __field( int, ret ) __field( int, pages_written ) __field( long, pages_skipped ) - __field( char, more_io )__field ( pgoff_t, writeback_index ) ),
@@ -315,15 +314,13 @@ TRACE_EVENT(ext4_da_writepages_result, __entry->ret = ret; __entry->pages_written = pages_written; __entry->pages_skipped = wbc->pages_skipped; - __entry->more_io = wbc->more_io; __entry->writeback_index = inode->i_mapping->writeback_index; ), -TP_printk ("dev %s ino %lu ret %d pages_written %d pages_skipped %ld more_io %d writeback_index %lu" , +TP_printk ("dev %s ino %lu ret %d pages_written %d pages_skipped %ld writeback_index %lu" ,jbd2_dev_to_name (__entry->dev), (unsigned long ) __entry->ino, __entry->ret, __entry->pages_written, __entry->pages_skipped, - __entry->more_io, (unsigned long ) __entry->writeback_index) );
Signature · 7 lines
1.7.1.1 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Wu Fengguang
AppliedQuoted 4 lines
Here's a touch up patch on top of your changes against the latest mmotm.
Signed-off-by Itaru Kitayama <[email protected]>
Applied, Thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>