All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] writeback: send work item to queue_io, move_expired_inodes
@ 2011-08-22 18:38 ` Curt Wohlgemuth
  0 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-22 18:38 UTC (permalink / raw)
  To: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton, Dave Chinner
  Cc: linux-fsdevel, linux-mm, Curt Wohlgemuth

Instead of sending ->older_than_this to queue_io() and
move_expired_inodes(), send the entire wb_writeback_work
structure.  There are other fields of a work item that are
useful in these routines and in tracepoints.

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---
 fs/fs-writeback.c                |   16 ++++++++--------
 include/trace/events/writeback.h |    5 +++--
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 04cf3b9..f1f5f65 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -251,7 +251,7 @@ static bool inode_dirtied_after(struct inode *inode, unsigned long t)
  */
 static int move_expired_inodes(struct list_head *delaying_queue,
 			       struct list_head *dispatch_queue,
-			       unsigned long *older_than_this)
+			       struct wb_writeback_work *work)
 {
 	LIST_HEAD(tmp);
 	struct list_head *pos, *node;
@@ -262,8 +262,8 @@ static int move_expired_inodes(struct list_head *delaying_queue,
 
 	while (!list_empty(delaying_queue)) {
 		inode = wb_inode(delaying_queue->prev);
-		if (older_than_this &&
-		    inode_dirtied_after(inode, *older_than_this))
+		if (work->older_than_this &&
+		    inode_dirtied_after(inode, *work->older_than_this))
 			break;
 		if (sb && sb != inode->i_sb)
 			do_sb_sort = 1;
@@ -302,13 +302,13 @@ out:
  *                                           |
  *                                           +--> dequeue for IO
  */
-static void queue_io(struct bdi_writeback *wb, unsigned long *older_than_this)
+static void queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work)
 {
 	int moved;
 	assert_spin_locked(&wb->list_lock);
 	list_splice_init(&wb->b_more_io, &wb->b_io);
-	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, older_than_this);
-	trace_writeback_queue_io(wb, older_than_this, moved);
+	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, work);
+	trace_writeback_queue_io(wb, work, moved);
 }
 
 static int write_inode(struct inode *inode, struct writeback_control *wbc)
@@ -651,7 +651,7 @@ long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
 
 	spin_lock(&wb->list_lock);
 	if (list_empty(&wb->b_io))
-		queue_io(wb, NULL);
+		queue_io(wb, &work);
 	__writeback_inodes_wb(wb, &work);
 	spin_unlock(&wb->list_lock);
 
@@ -738,7 +738,7 @@ static long wb_writeback(struct bdi_writeback *wb,
 
 		trace_writeback_start(wb->bdi, work);
 		if (list_empty(&wb->b_io))
-			queue_io(wb, work->older_than_this);
+			queue_io(wb, work);
 		if (work->sb)
 			progress = writeback_sb_inodes(work->sb, wb, work);
 		else
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 6bca4cc..4565274 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -181,9 +181,9 @@ DEFINE_WBC_EVENT(wbc_writepage);
 
 TRACE_EVENT(writeback_queue_io,
 	TP_PROTO(struct bdi_writeback *wb,
-		 unsigned long *older_than_this,
+		 struct wb_writeback_work *work,
 		 int moved),
-	TP_ARGS(wb, older_than_this, moved),
+	TP_ARGS(wb, work, moved),
 	TP_STRUCT__entry(
 		__array(char,		name, 32)
 		__field(unsigned long,	older)
@@ -191,6 +191,7 @@ TRACE_EVENT(writeback_queue_io,
 		__field(int,		moved)
 	),
 	TP_fast_assign(
+		unsigned long *older_than_this = work->older_than_this;
 		strncpy(__entry->name, dev_name(wb->bdi->dev), 32);
 		__entry->older	= older_than_this ?  *older_than_this : 0;
 		__entry->age	= older_than_this ?
-- 
1.7.3.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 1/3] writeback: send work item to queue_io, move_expired_inodes
@ 2011-08-22 18:38 ` Curt Wohlgemuth
  0 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-22 18:38 UTC (permalink / raw)
  To: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin
  Cc: linux-fsdevel, linux-mm, Curt Wohlgemuth

Instead of sending ->older_than_this to queue_io() and
move_expired_inodes(), send the entire wb_writeback_work
structure.  There are other fields of a work item that are
useful in these routines and in tracepoints.

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---
 fs/fs-writeback.c                |   16 ++++++++--------
 include/trace/events/writeback.h |    5 +++--
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 04cf3b9..f1f5f65 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -251,7 +251,7 @@ static bool inode_dirtied_after(struct inode *inode, unsigned long t)
  */
 static int move_expired_inodes(struct list_head *delaying_queue,
 			       struct list_head *dispatch_queue,
-			       unsigned long *older_than_this)
+			       struct wb_writeback_work *work)
 {
 	LIST_HEAD(tmp);
 	struct list_head *pos, *node;
@@ -262,8 +262,8 @@ static int move_expired_inodes(struct list_head *delaying_queue,
 
 	while (!list_empty(delaying_queue)) {
 		inode = wb_inode(delaying_queue->prev);
-		if (older_than_this &&
-		    inode_dirtied_after(inode, *older_than_this))
+		if (work->older_than_this &&
+		    inode_dirtied_after(inode, *work->older_than_this))
 			break;
 		if (sb && sb != inode->i_sb)
 			do_sb_sort = 1;
@@ -302,13 +302,13 @@ out:
  *                                           |
  *                                           +--> dequeue for IO
  */
-static void queue_io(struct bdi_writeback *wb, unsigned long *older_than_this)
+static void queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work)
 {
 	int moved;
 	assert_spin_locked(&wb->list_lock);
 	list_splice_init(&wb->b_more_io, &wb->b_io);
-	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, older_than_this);
-	trace_writeback_queue_io(wb, older_than_this, moved);
+	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, work);
+	trace_writeback_queue_io(wb, work, moved);
 }
 
 static int write_inode(struct inode *inode, struct writeback_control *wbc)
@@ -651,7 +651,7 @@ long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
 
 	spin_lock(&wb->list_lock);
 	if (list_empty(&wb->b_io))
-		queue_io(wb, NULL);
+		queue_io(wb, &work);
 	__writeback_inodes_wb(wb, &work);
 	spin_unlock(&wb->list_lock);
 
@@ -738,7 +738,7 @@ static long wb_writeback(struct bdi_writeback *wb,
 
 		trace_writeback_start(wb->bdi, work);
 		if (list_empty(&wb->b_io))
-			queue_io(wb, work->older_than_this);
+			queue_io(wb, work);
 		if (work->sb)
 			progress = writeback_sb_inodes(work->sb, wb, work);
 		else
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 6bca4cc..4565274 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -181,9 +181,9 @@ DEFINE_WBC_EVENT(wbc_writepage);
 
 TRACE_EVENT(writeback_queue_io,
 	TP_PROTO(struct bdi_writeback *wb,
-		 unsigned long *older_than_this,
+		 struct wb_writeback_work *work,
 		 int moved),
-	TP_ARGS(wb, older_than_this, moved),
+	TP_ARGS(wb, work, moved),
 	TP_STRUCT__entry(
 		__array(char,		name, 32)
 		__field(unsigned long,	older)
@@ -191,6 +191,7 @@ TRACE_EVENT(writeback_queue_io,
 		__field(int,		moved)
 	),
 	TP_fast_assign(
+		unsigned long *older_than_this = work->older_than_this;
 		strncpy(__entry->name, dev_name(wb->bdi->dev), 32);
 		__entry->older	= older_than_this ?  *older_than_this : 0;
 		__entry->age	= older_than_this ?
-- 
1.7.3.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work
  2011-08-22 18:38 ` Curt Wohlgemuth
@ 2011-08-22 18:38   ` Curt Wohlgemuth
  -1 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-22 18:38 UTC (permalink / raw)
  To: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton, Dave Chinner
  Cc: linux-fsdevel, linux-mm, Curt Wohlgemuth

This creates a new 'reason' field in a wb_writeback_work
structure, which unambiguously identifies who initiates
writeback activity.  A 'wb_reason' enumeration has been
added to writeback.h, to enumerate the possible reasons.

The 'writeback_work_class' and tracepoint event class and
'writeback_queue_io' tracepoints are updated to include the
symbolic 'reason' in all trace events.

And the 'writeback_inodes_sbXXX' family of routines has had
a wb_stats parameter added to them, so callers can specify
why writeback is being started.

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---

Changes since v2:

   - enum wb_stats renamed enum wb_reason
   - All WB_STAT_xxx enum constants now named WB_REASON_xxx
   - All 'reason' strings in tracepoints match the WB_REASON name
   - The change to send 'work' to queue_io, move_expired_inodes, and
     trace_writeback_queue_io are in a separate commit


 fs/btrfs/extent-tree.c           |    3 ++-
 fs/buffer.c                      |    2 +-
 fs/ext4/inode.c                  |    2 +-
 fs/fs-writeback.c                |   38 +++++++++++++++++++++++++-------------
 fs/quota/quota.c                 |    2 +-
 fs/sync.c                        |    4 ++--
 fs/ubifs/budget.c                |    2 +-
 include/linux/backing-dev.h      |    3 ++-
 include/linux/writeback.h        |   33 +++++++++++++++++++++++++++------
 include/trace/events/writeback.h |   27 +++++++++++++++++++++++----
 mm/backing-dev.c                 |    3 ++-
 mm/page-writeback.c              |    6 ++++--
 mm/vmscan.c                      |    4 ++--
 13 files changed, 93 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f5be06a..c9ee0e1 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3340,7 +3340,8 @@ static int shrink_delalloc(struct btrfs_trans_handle *trans,
 		smp_mb();
 		nr_pages = min_t(unsigned long, nr_pages,
 		       root->fs_info->delalloc_bytes >> PAGE_CACHE_SHIFT);
-		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages);
+		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages,
+						WB_REASON_FS_FREE_SPACE);
 
 		spin_lock(&space_info->lock);
 		if (reserved > space_info->bytes_reserved)
diff --git a/fs/buffer.c b/fs/buffer.c
index 1a80b04..f5dcee6 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -285,7 +285,7 @@ static void free_more_memory(void)
 	struct zone *zone;
 	int nid;
 
-	wakeup_flusher_threads(1024);
+	wakeup_flusher_threads(1024, WB_REASON_FREE_MORE_MEM);
 	yield();
 
 	for_each_online_node(nid) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c4da98a..f4f0f40 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2244,7 +2244,7 @@ static int ext4_nonda_switch(struct super_block *sb)
 	 * start pushing delalloc when 1/2 of free blocks are dirty.
 	 */
 	if (free_blocks < 2 * dirty_blocks)
-		writeback_inodes_sb_if_idle(sb);
+		writeback_inodes_sb_if_idle(sb, WB_REASON_FS_FREE_SPACE);
 
 	return 0;
 }
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index f1f5f65..a004fcd 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -41,6 +41,7 @@ struct wb_writeback_work {
 	unsigned int for_kupdate:1;
 	unsigned int range_cyclic:1;
 	unsigned int for_background:1;
+	enum wb_reason reason;		/* why was writeback initiated? */
 
 	struct list_head list;		/* pending work list */
 	struct completion *done;	/* set if the caller waits */
@@ -115,7 +116,7 @@ static void bdi_queue_work(struct backing_dev_info *bdi,
 
 static void
 __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
-		      bool range_cyclic)
+		      bool range_cyclic, enum wb_reason reason)
 {
 	struct wb_writeback_work *work;
 
@@ -135,6 +136,7 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
 	work->sync_mode	= WB_SYNC_NONE;
 	work->nr_pages	= nr_pages;
 	work->range_cyclic = range_cyclic;
+	work->reason = reason;
 
 	bdi_queue_work(bdi, work);
 }
@@ -150,9 +152,10 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
  *   completion. Caller need not hold sb s_umount semaphore.
  *
  */
-void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages)
+void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
+			enum wb_reason reason)
 {
-	__bdi_start_writeback(bdi, nr_pages, true);
+	__bdi_start_writeback(bdi, nr_pages, true, reason);
 }
 
 /**
@@ -641,12 +644,14 @@ static long __writeback_inodes_wb(struct bdi_writeback *wb,
 	return wrote;
 }
 
-long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
+long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
+				enum wb_reason reason)
 {
 	struct wb_writeback_work work = {
 		.nr_pages	= nr_pages,
 		.sync_mode	= WB_SYNC_NONE,
 		.range_cyclic	= 1,
+		.reason		= reason,
 	};
 
 	spin_lock(&wb->list_lock);
@@ -818,6 +823,7 @@ static long wb_check_background_flush(struct bdi_writeback *wb)
 			.sync_mode	= WB_SYNC_NONE,
 			.for_background	= 1,
 			.range_cyclic	= 1,
+			.reason		= WB_REASON_BACKGROUND,
 		};
 
 		return wb_writeback(wb, &work);
@@ -851,6 +857,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
 			.sync_mode	= WB_SYNC_NONE,
 			.for_kupdate	= 1,
 			.range_cyclic	= 1,
+			.reason		= WB_REASON_PERIODIC,
 		};
 
 		return wb_writeback(wb, &work);
@@ -969,7 +976,7 @@ int bdi_writeback_thread(void *data)
  * Start writeback of `nr_pages' pages.  If `nr_pages' is zero, write back
  * the whole world.
  */
-void wakeup_flusher_threads(long nr_pages)
+void wakeup_flusher_threads(long nr_pages, enum wb_reason reason)
 {
 	struct backing_dev_info *bdi;
 
@@ -982,7 +989,7 @@ void wakeup_flusher_threads(long nr_pages)
 	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) {
 		if (!bdi_has_dirty_io(bdi))
 			continue;
-		__bdi_start_writeback(bdi, nr_pages, false);
+		__bdi_start_writeback(bdi, nr_pages, false, reason);
 	}
 	rcu_read_unlock();
 }
@@ -1203,7 +1210,9 @@ static void wait_sb_inodes(struct super_block *sb)
  * on how many (if any) will be written, and this function does not wait
  * for IO completion of submitted IO.
  */
-void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
+void writeback_inodes_sb_nr(struct super_block *sb,
+			    unsigned long nr,
+			    enum wb_reason reason)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
 	struct wb_writeback_work work = {
@@ -1212,6 +1221,7 @@ void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
 		.tagged_writepages	= 1,
 		.done			= &done,
 		.nr_pages		= nr,
+		.reason			= reason,
 	};
 
 	WARN_ON(!rwsem_is_locked(&sb->s_umount));
@@ -1228,9 +1238,9 @@ EXPORT_SYMBOL(writeback_inodes_sb_nr);
  * on how many (if any) will be written, and this function does not wait
  * for IO completion of submitted IO.
  */
-void writeback_inodes_sb(struct super_block *sb)
+void writeback_inodes_sb(struct super_block *sb, enum wb_reason reason)
 {
-	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages());
+	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages(), reason);
 }
 EXPORT_SYMBOL(writeback_inodes_sb);
 
@@ -1241,11 +1251,11 @@ EXPORT_SYMBOL(writeback_inodes_sb);
  * Invoke writeback_inodes_sb if no writeback is currently underway.
  * Returns 1 if writeback was started, 0 if not.
  */
-int writeback_inodes_sb_if_idle(struct super_block *sb)
+int writeback_inodes_sb_if_idle(struct super_block *sb, enum wb_reason reason)
 {
 	if (!writeback_in_progress(sb->s_bdi)) {
 		down_read(&sb->s_umount);
-		writeback_inodes_sb(sb);
+		writeback_inodes_sb(sb, reason);
 		up_read(&sb->s_umount);
 		return 1;
 	} else
@@ -1262,11 +1272,12 @@ EXPORT_SYMBOL(writeback_inodes_sb_if_idle);
  * Returns 1 if writeback was started, 0 if not.
  */
 int writeback_inodes_sb_nr_if_idle(struct super_block *sb,
-				   unsigned long nr)
+				   unsigned long nr,
+				   enum wb_reason reason)
 {
 	if (!writeback_in_progress(sb->s_bdi)) {
 		down_read(&sb->s_umount);
-		writeback_inodes_sb_nr(sb, nr);
+		writeback_inodes_sb_nr(sb, nr, reason);
 		up_read(&sb->s_umount);
 		return 1;
 	} else
@@ -1290,6 +1301,7 @@ void sync_inodes_sb(struct super_block *sb)
 		.nr_pages	= LONG_MAX,
 		.range_cyclic	= 0,
 		.done		= &done,
+		.reason		= WB_REASON_SYNC,
 	};
 
 	WARN_ON(!rwsem_is_locked(&sb->s_umount));
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index b34bdb2..b7a4366 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -286,7 +286,7 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 		/* caller already holds s_umount */
 		if (sb->s_flags & MS_RDONLY)
 			return -EROFS;
-		writeback_inodes_sb(sb);
+		writeback_inodes_sb(sb, WB_REASON_SYNC);
 		return 0;
 	default:
 		return -EINVAL;
diff --git a/fs/sync.c b/fs/sync.c
index c98a747..101b8ef 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -43,7 +43,7 @@ static int __sync_filesystem(struct super_block *sb, int wait)
 	if (wait)
 		sync_inodes_sb(sb);
 	else
-		writeback_inodes_sb(sb);
+		writeback_inodes_sb(sb, WB_REASON_SYNC);
 
 	if (sb->s_op->sync_fs)
 		sb->s_op->sync_fs(sb, wait);
@@ -98,7 +98,7 @@ static void sync_filesystems(int wait)
  */
 SYSCALL_DEFINE0(sync)
 {
-	wakeup_flusher_threads(0);
+	wakeup_flusher_threads(0, WB_REASON_SYNC);
 	sync_filesystems(0);
 	sync_filesystems(1);
 	if (unlikely(laptop_mode))
diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index 315de66..bc4f94b 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -63,7 +63,7 @@
 static void shrink_liability(struct ubifs_info *c, int nr_to_write)
 {
 	down_read(&c->vfs_sb->s_umount);
-	writeback_inodes_sb(c->vfs_sb);
+	writeback_inodes_sb(c->vfs_sb, WB_REASON_FS_FREE_SPACE);
 	up_read(&c->vfs_sb->s_umount);
 }
 
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 3b2f9cb..ef85559 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -107,7 +107,8 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent,
 int bdi_register_dev(struct backing_dev_info *bdi, dev_t dev);
 void bdi_unregister(struct backing_dev_info *bdi);
 int bdi_setup_and_register(struct backing_dev_info *, char *, unsigned int);
-void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages);
+void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
+			enum wb_reason stat);
 void bdi_start_background_writeback(struct backing_dev_info *bdi);
 int bdi_writeback_thread(void *data);
 int bdi_has_dirty_io(struct backing_dev_info *bdi);
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index f1bfa12e..bdda069 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -50,6 +50,24 @@ enum writeback_sync_modes {
 };
 
 /*
+ * why this writeback was initiated
+ */
+enum wb_reason {
+	/* The following are counts of pages written for a specific cause */
+	WB_REASON_BALANCE_DIRTY,
+	WB_REASON_BACKGROUND,
+	WB_REASON_TRY_TO_FREE_PAGES,
+	WB_REASON_SYNC,
+	WB_REASON_PERIODIC,
+	WB_REASON_LAPTOP_TIMER,
+	WB_REASON_FREE_MORE_MEM,
+	WB_REASON_FS_FREE_SPACE,
+	WB_REASON_FORKER_THREAD,
+
+	WB_REASON_MAX,
+};
+
+/*
  * A control structure which tells the writeback code what to do.  These are
  * always on the stack, and hence need no locking.  They are always initialised
  * in a manner such that unspecified fields are set to zero.
@@ -80,14 +98,17 @@ struct writeback_control {
  */	
 struct bdi_writeback;
 int inode_wait(void *);
-void writeback_inodes_sb(struct super_block *);
-void writeback_inodes_sb_nr(struct super_block *, unsigned long nr);
-int writeback_inodes_sb_if_idle(struct super_block *);
-int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr);
+void writeback_inodes_sb(struct super_block *, enum wb_reason stat);
+void writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
+							enum wb_reason stat);
+int writeback_inodes_sb_if_idle(struct super_block *, enum wb_reason stat);
+int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr,
+							enum wb_reason stat);
 void sync_inodes_sb(struct super_block *);
-long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages);
+long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
+				enum wb_reason stat);
 long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
-void wakeup_flusher_threads(long nr_pages);
+void wakeup_flusher_threads(long nr_pages, enum wb_reason stat);
 
 /* writeback.h requires fs.h; it, too, is not included from here. */
 static inline void wait_on_inode(struct inode *inode)
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 4565274..b8583cb 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -21,6 +21,19 @@
 		{I_REFERENCED,		"I_REFERENCED"}		\
 	)
 
+#define show_work_reason(reason)					\
+	__print_symbolic(reason,					\
+		{WB_REASON_BALANCE_DIRTY,	"balance_dirty"},	\
+		{WB_REASON_BACKGROUND,		"background"},		\
+		{WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"},	\
+		{WB_REASON_SYNC,		"sync"},		\
+		{WB_REASON_PERIODIC,		"periodic"},		\
+		{WB_REASON_LAPTOP_TIMER,	"laptop_timer"},	\
+		{WB_REASON_FREE_MORE_MEM,	"free_more_memory"},	\
+		{WB_REASON_FS_FREE_SPACE,	"FS_free_space"},	\
+		{WB_REASON_FORKER_THREAD,	"forker_thread"}	\
+	)
+
 struct wb_writeback_work;
 
 DECLARE_EVENT_CLASS(writeback_work_class,
@@ -34,6 +47,7 @@ DECLARE_EVENT_CLASS(writeback_work_class,
 		__field(int, for_kupdate)
 		__field(int, range_cyclic)
 		__field(int, for_background)
+		__field(int, reason)
 	),
 	TP_fast_assign(
 		strncpy(__entry->name, dev_name(bdi->dev), 32);
@@ -43,16 +57,18 @@ DECLARE_EVENT_CLASS(writeback_work_class,
 		__entry->for_kupdate = work->for_kupdate;
 		__entry->range_cyclic = work->range_cyclic;
 		__entry->for_background	= work->for_background;
+		__entry->reason = work->reason;
 	),
 	TP_printk("bdi %s: sb_dev %d:%d nr_pages=%ld sync_mode=%d "
-		  "kupdate=%d range_cyclic=%d background=%d",
+		  "kupdate=%d range_cyclic=%d background=%d reason=%s",
 		  __entry->name,
 		  MAJOR(__entry->sb_dev), MINOR(__entry->sb_dev),
 		  __entry->nr_pages,
 		  __entry->sync_mode,
 		  __entry->for_kupdate,
 		  __entry->range_cyclic,
-		  __entry->for_background
+		  __entry->for_background,
+		  show_work_reason(__entry->reason)
 	)
 );
 #define DEFINE_WRITEBACK_WORK_EVENT(name) \
@@ -189,6 +205,7 @@ TRACE_EVENT(writeback_queue_io,
 		__field(unsigned long,	older)
 		__field(long,		age)
 		__field(int,		moved)
+		__field(int,		reason)
 	),
 	TP_fast_assign(
 		unsigned long *older_than_this = work->older_than_this;
@@ -197,12 +214,14 @@ TRACE_EVENT(writeback_queue_io,
 		__entry->age	= older_than_this ?
 				  (jiffies - *older_than_this) * 1000 / HZ : -1;
 		__entry->moved	= moved;
+		__entry->reason	= work->reason;
 	),
-	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d",
+	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d reason=%s",
 		__entry->name,
 		__entry->older,	/* older_than_this in jiffies */
 		__entry->age,	/* older_than_this in relative milliseconds */
-		__entry->moved)
+		__entry->moved,
+		show_work_reason(__entry->reason))
 );
 
 TRACE_EVENT(global_dirty_state,
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index d6edf8d..474bcfe 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -456,7 +456,8 @@ static int bdi_forker_thread(void *ptr)
 				 * the bdi from the thread. Hopefully 1024 is
 				 * large enough for efficient IO.
 				 */
-				writeback_inodes_wb(&bdi->wb, 1024);
+				writeback_inodes_wb(&bdi->wb, 1024,
+						    WB_REASON_FORKER_THREAD);
 			} else {
 				/*
 				 * The spinlock makes sure we do not lose
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index d196074..0e78252 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -738,7 +738,8 @@ static void balance_dirty_pages(struct address_space *mapping,
 		trace_balance_dirty_start(bdi);
 		if (bdi_nr_reclaimable > task_bdi_thresh) {
 			pages_written += writeback_inodes_wb(&bdi->wb,
-							     write_chunk);
+						write_chunk,
+						WB_REASON_BALANCE_DIRTY);
 			trace_balance_dirty_written(bdi, pages_written);
 			if (pages_written >= write_chunk)
 				break;		/* We've done our duty */
@@ -909,7 +910,8 @@ void laptop_mode_timer_fn(unsigned long data)
 	 * threshold
 	 */
 	if (bdi_has_dirty_io(&q->backing_dev_info))
-		bdi_start_writeback(&q->backing_dev_info, nr_pages);
+		bdi_start_writeback(&q->backing_dev_info, nr_pages,
+					WB_REASON_LAPTOP_TIMER);
 }
 
 /*
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7ef6912..3b6c7ac 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -495,7 +495,6 @@ static pageout_t pageout(struct page *page, struct address_space *mapping,
 			ClearPageReclaim(page);
 			return PAGE_ACTIVATE;
 		}
-
 		/*
 		 * Wait on writeback if requested to. This happens when
 		 * direct reclaiming a large contiguous area and the
@@ -2198,7 +2197,8 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 		 */
 		writeback_threshold = sc->nr_to_reclaim + sc->nr_to_reclaim / 2;
 		if (total_scanned > writeback_threshold) {
-			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned);
+			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned,
+						WB_REASON_TRY_TO_FREE_PAGES);
 			sc->may_writepage = 1;
 		}
 
-- 
1.7.3.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work
@ 2011-08-22 18:38   ` Curt Wohlgemuth
  0 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-22 18:38 UTC (permalink / raw)
  To: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin
  Cc: linux-fsdevel, linux-mm, Curt Wohlgemuth

This creates a new 'reason' field in a wb_writeback_work
structure, which unambiguously identifies who initiates
writeback activity.  A 'wb_reason' enumeration has been
added to writeback.h, to enumerate the possible reasons.

The 'writeback_work_class' and tracepoint event class and
'writeback_queue_io' tracepoints are updated to include the
symbolic 'reason' in all trace events.

And the 'writeback_inodes_sbXXX' family of routines has had
a wb_stats parameter added to them, so callers can specify
why writeback is being started.

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---

Changes since v2:

   - enum wb_stats renamed enum wb_reason
   - All WB_STAT_xxx enum constants now named WB_REASON_xxx
   - All 'reason' strings in tracepoints match the WB_REASON name
   - The change to send 'work' to queue_io, move_expired_inodes, and
     trace_writeback_queue_io are in a separate commit


 fs/btrfs/extent-tree.c           |    3 ++-
 fs/buffer.c                      |    2 +-
 fs/ext4/inode.c                  |    2 +-
 fs/fs-writeback.c                |   38 +++++++++++++++++++++++++-------------
 fs/quota/quota.c                 |    2 +-
 fs/sync.c                        |    4 ++--
 fs/ubifs/budget.c                |    2 +-
 include/linux/backing-dev.h      |    3 ++-
 include/linux/writeback.h        |   33 +++++++++++++++++++++++++++------
 include/trace/events/writeback.h |   27 +++++++++++++++++++++++----
 mm/backing-dev.c                 |    3 ++-
 mm/page-writeback.c              |    6 ++++--
 mm/vmscan.c                      |    4 ++--
 13 files changed, 93 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f5be06a..c9ee0e1 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3340,7 +3340,8 @@ static int shrink_delalloc(struct btrfs_trans_handle *trans,
 		smp_mb();
 		nr_pages = min_t(unsigned long, nr_pages,
 		       root->fs_info->delalloc_bytes >> PAGE_CACHE_SHIFT);
-		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages);
+		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages,
+						WB_REASON_FS_FREE_SPACE);
 
 		spin_lock(&space_info->lock);
 		if (reserved > space_info->bytes_reserved)
diff --git a/fs/buffer.c b/fs/buffer.c
index 1a80b04..f5dcee6 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -285,7 +285,7 @@ static void free_more_memory(void)
 	struct zone *zone;
 	int nid;
 
-	wakeup_flusher_threads(1024);
+	wakeup_flusher_threads(1024, WB_REASON_FREE_MORE_MEM);
 	yield();
 
 	for_each_online_node(nid) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c4da98a..f4f0f40 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2244,7 +2244,7 @@ static int ext4_nonda_switch(struct super_block *sb)
 	 * start pushing delalloc when 1/2 of free blocks are dirty.
 	 */
 	if (free_blocks < 2 * dirty_blocks)
-		writeback_inodes_sb_if_idle(sb);
+		writeback_inodes_sb_if_idle(sb, WB_REASON_FS_FREE_SPACE);
 
 	return 0;
 }
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index f1f5f65..a004fcd 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -41,6 +41,7 @@ struct wb_writeback_work {
 	unsigned int for_kupdate:1;
 	unsigned int range_cyclic:1;
 	unsigned int for_background:1;
+	enum wb_reason reason;		/* why was writeback initiated? */
 
 	struct list_head list;		/* pending work list */
 	struct completion *done;	/* set if the caller waits */
@@ -115,7 +116,7 @@ static void bdi_queue_work(struct backing_dev_info *bdi,
 
 static void
 __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
-		      bool range_cyclic)
+		      bool range_cyclic, enum wb_reason reason)
 {
 	struct wb_writeback_work *work;
 
@@ -135,6 +136,7 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
 	work->sync_mode	= WB_SYNC_NONE;
 	work->nr_pages	= nr_pages;
 	work->range_cyclic = range_cyclic;
+	work->reason = reason;
 
 	bdi_queue_work(bdi, work);
 }
@@ -150,9 +152,10 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
  *   completion. Caller need not hold sb s_umount semaphore.
  *
  */
-void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages)
+void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
+			enum wb_reason reason)
 {
-	__bdi_start_writeback(bdi, nr_pages, true);
+	__bdi_start_writeback(bdi, nr_pages, true, reason);
 }
 
 /**
@@ -641,12 +644,14 @@ static long __writeback_inodes_wb(struct bdi_writeback *wb,
 	return wrote;
 }
 
-long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
+long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
+				enum wb_reason reason)
 {
 	struct wb_writeback_work work = {
 		.nr_pages	= nr_pages,
 		.sync_mode	= WB_SYNC_NONE,
 		.range_cyclic	= 1,
+		.reason		= reason,
 	};
 
 	spin_lock(&wb->list_lock);
@@ -818,6 +823,7 @@ static long wb_check_background_flush(struct bdi_writeback *wb)
 			.sync_mode	= WB_SYNC_NONE,
 			.for_background	= 1,
 			.range_cyclic	= 1,
+			.reason		= WB_REASON_BACKGROUND,
 		};
 
 		return wb_writeback(wb, &work);
@@ -851,6 +857,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
 			.sync_mode	= WB_SYNC_NONE,
 			.for_kupdate	= 1,
 			.range_cyclic	= 1,
+			.reason		= WB_REASON_PERIODIC,
 		};
 
 		return wb_writeback(wb, &work);
@@ -969,7 +976,7 @@ int bdi_writeback_thread(void *data)
  * Start writeback of `nr_pages' pages.  If `nr_pages' is zero, write back
  * the whole world.
  */
-void wakeup_flusher_threads(long nr_pages)
+void wakeup_flusher_threads(long nr_pages, enum wb_reason reason)
 {
 	struct backing_dev_info *bdi;
 
@@ -982,7 +989,7 @@ void wakeup_flusher_threads(long nr_pages)
 	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) {
 		if (!bdi_has_dirty_io(bdi))
 			continue;
-		__bdi_start_writeback(bdi, nr_pages, false);
+		__bdi_start_writeback(bdi, nr_pages, false, reason);
 	}
 	rcu_read_unlock();
 }
@@ -1203,7 +1210,9 @@ static void wait_sb_inodes(struct super_block *sb)
  * on how many (if any) will be written, and this function does not wait
  * for IO completion of submitted IO.
  */
-void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
+void writeback_inodes_sb_nr(struct super_block *sb,
+			    unsigned long nr,
+			    enum wb_reason reason)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
 	struct wb_writeback_work work = {
@@ -1212,6 +1221,7 @@ void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
 		.tagged_writepages	= 1,
 		.done			= &done,
 		.nr_pages		= nr,
+		.reason			= reason,
 	};
 
 	WARN_ON(!rwsem_is_locked(&sb->s_umount));
@@ -1228,9 +1238,9 @@ EXPORT_SYMBOL(writeback_inodes_sb_nr);
  * on how many (if any) will be written, and this function does not wait
  * for IO completion of submitted IO.
  */
-void writeback_inodes_sb(struct super_block *sb)
+void writeback_inodes_sb(struct super_block *sb, enum wb_reason reason)
 {
-	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages());
+	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages(), reason);
 }
 EXPORT_SYMBOL(writeback_inodes_sb);
 
@@ -1241,11 +1251,11 @@ EXPORT_SYMBOL(writeback_inodes_sb);
  * Invoke writeback_inodes_sb if no writeback is currently underway.
  * Returns 1 if writeback was started, 0 if not.
  */
-int writeback_inodes_sb_if_idle(struct super_block *sb)
+int writeback_inodes_sb_if_idle(struct super_block *sb, enum wb_reason reason)
 {
 	if (!writeback_in_progress(sb->s_bdi)) {
 		down_read(&sb->s_umount);
-		writeback_inodes_sb(sb);
+		writeback_inodes_sb(sb, reason);
 		up_read(&sb->s_umount);
 		return 1;
 	} else
@@ -1262,11 +1272,12 @@ EXPORT_SYMBOL(writeback_inodes_sb_if_idle);
  * Returns 1 if writeback was started, 0 if not.
  */
 int writeback_inodes_sb_nr_if_idle(struct super_block *sb,
-				   unsigned long nr)
+				   unsigned long nr,
+				   enum wb_reason reason)
 {
 	if (!writeback_in_progress(sb->s_bdi)) {
 		down_read(&sb->s_umount);
-		writeback_inodes_sb_nr(sb, nr);
+		writeback_inodes_sb_nr(sb, nr, reason);
 		up_read(&sb->s_umount);
 		return 1;
 	} else
@@ -1290,6 +1301,7 @@ void sync_inodes_sb(struct super_block *sb)
 		.nr_pages	= LONG_MAX,
 		.range_cyclic	= 0,
 		.done		= &done,
+		.reason		= WB_REASON_SYNC,
 	};
 
 	WARN_ON(!rwsem_is_locked(&sb->s_umount));
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index b34bdb2..b7a4366 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -286,7 +286,7 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 		/* caller already holds s_umount */
 		if (sb->s_flags & MS_RDONLY)
 			return -EROFS;
-		writeback_inodes_sb(sb);
+		writeback_inodes_sb(sb, WB_REASON_SYNC);
 		return 0;
 	default:
 		return -EINVAL;
diff --git a/fs/sync.c b/fs/sync.c
index c98a747..101b8ef 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -43,7 +43,7 @@ static int __sync_filesystem(struct super_block *sb, int wait)
 	if (wait)
 		sync_inodes_sb(sb);
 	else
-		writeback_inodes_sb(sb);
+		writeback_inodes_sb(sb, WB_REASON_SYNC);
 
 	if (sb->s_op->sync_fs)
 		sb->s_op->sync_fs(sb, wait);
@@ -98,7 +98,7 @@ static void sync_filesystems(int wait)
  */
 SYSCALL_DEFINE0(sync)
 {
-	wakeup_flusher_threads(0);
+	wakeup_flusher_threads(0, WB_REASON_SYNC);
 	sync_filesystems(0);
 	sync_filesystems(1);
 	if (unlikely(laptop_mode))
diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index 315de66..bc4f94b 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -63,7 +63,7 @@
 static void shrink_liability(struct ubifs_info *c, int nr_to_write)
 {
 	down_read(&c->vfs_sb->s_umount);
-	writeback_inodes_sb(c->vfs_sb);
+	writeback_inodes_sb(c->vfs_sb, WB_REASON_FS_FREE_SPACE);
 	up_read(&c->vfs_sb->s_umount);
 }
 
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 3b2f9cb..ef85559 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -107,7 +107,8 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent,
 int bdi_register_dev(struct backing_dev_info *bdi, dev_t dev);
 void bdi_unregister(struct backing_dev_info *bdi);
 int bdi_setup_and_register(struct backing_dev_info *, char *, unsigned int);
-void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages);
+void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
+			enum wb_reason stat);
 void bdi_start_background_writeback(struct backing_dev_info *bdi);
 int bdi_writeback_thread(void *data);
 int bdi_has_dirty_io(struct backing_dev_info *bdi);
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index f1bfa12e..bdda069 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -50,6 +50,24 @@ enum writeback_sync_modes {
 };
 
 /*
+ * why this writeback was initiated
+ */
+enum wb_reason {
+	/* The following are counts of pages written for a specific cause */
+	WB_REASON_BALANCE_DIRTY,
+	WB_REASON_BACKGROUND,
+	WB_REASON_TRY_TO_FREE_PAGES,
+	WB_REASON_SYNC,
+	WB_REASON_PERIODIC,
+	WB_REASON_LAPTOP_TIMER,
+	WB_REASON_FREE_MORE_MEM,
+	WB_REASON_FS_FREE_SPACE,
+	WB_REASON_FORKER_THREAD,
+
+	WB_REASON_MAX,
+};
+
+/*
  * A control structure which tells the writeback code what to do.  These are
  * always on the stack, and hence need no locking.  They are always initialised
  * in a manner such that unspecified fields are set to zero.
@@ -80,14 +98,17 @@ struct writeback_control {
  */	
 struct bdi_writeback;
 int inode_wait(void *);
-void writeback_inodes_sb(struct super_block *);
-void writeback_inodes_sb_nr(struct super_block *, unsigned long nr);
-int writeback_inodes_sb_if_idle(struct super_block *);
-int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr);
+void writeback_inodes_sb(struct super_block *, enum wb_reason stat);
+void writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
+							enum wb_reason stat);
+int writeback_inodes_sb_if_idle(struct super_block *, enum wb_reason stat);
+int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr,
+							enum wb_reason stat);
 void sync_inodes_sb(struct super_block *);
-long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages);
+long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
+				enum wb_reason stat);
 long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
-void wakeup_flusher_threads(long nr_pages);
+void wakeup_flusher_threads(long nr_pages, enum wb_reason stat);
 
 /* writeback.h requires fs.h; it, too, is not included from here. */
 static inline void wait_on_inode(struct inode *inode)
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 4565274..b8583cb 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -21,6 +21,19 @@
 		{I_REFERENCED,		"I_REFERENCED"}		\
 	)
 
+#define show_work_reason(reason)					\
+	__print_symbolic(reason,					\
+		{WB_REASON_BALANCE_DIRTY,	"balance_dirty"},	\
+		{WB_REASON_BACKGROUND,		"background"},		\
+		{WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"},	\
+		{WB_REASON_SYNC,		"sync"},		\
+		{WB_REASON_PERIODIC,		"periodic"},		\
+		{WB_REASON_LAPTOP_TIMER,	"laptop_timer"},	\
+		{WB_REASON_FREE_MORE_MEM,	"free_more_memory"},	\
+		{WB_REASON_FS_FREE_SPACE,	"FS_free_space"},	\
+		{WB_REASON_FORKER_THREAD,	"forker_thread"}	\
+	)
+
 struct wb_writeback_work;
 
 DECLARE_EVENT_CLASS(writeback_work_class,
@@ -34,6 +47,7 @@ DECLARE_EVENT_CLASS(writeback_work_class,
 		__field(int, for_kupdate)
 		__field(int, range_cyclic)
 		__field(int, for_background)
+		__field(int, reason)
 	),
 	TP_fast_assign(
 		strncpy(__entry->name, dev_name(bdi->dev), 32);
@@ -43,16 +57,18 @@ DECLARE_EVENT_CLASS(writeback_work_class,
 		__entry->for_kupdate = work->for_kupdate;
 		__entry->range_cyclic = work->range_cyclic;
 		__entry->for_background	= work->for_background;
+		__entry->reason = work->reason;
 	),
 	TP_printk("bdi %s: sb_dev %d:%d nr_pages=%ld sync_mode=%d "
-		  "kupdate=%d range_cyclic=%d background=%d",
+		  "kupdate=%d range_cyclic=%d background=%d reason=%s",
 		  __entry->name,
 		  MAJOR(__entry->sb_dev), MINOR(__entry->sb_dev),
 		  __entry->nr_pages,
 		  __entry->sync_mode,
 		  __entry->for_kupdate,
 		  __entry->range_cyclic,
-		  __entry->for_background
+		  __entry->for_background,
+		  show_work_reason(__entry->reason)
 	)
 );
 #define DEFINE_WRITEBACK_WORK_EVENT(name) \
@@ -189,6 +205,7 @@ TRACE_EVENT(writeback_queue_io,
 		__field(unsigned long,	older)
 		__field(long,		age)
 		__field(int,		moved)
+		__field(int,		reason)
 	),
 	TP_fast_assign(
 		unsigned long *older_than_this = work->older_than_this;
@@ -197,12 +214,14 @@ TRACE_EVENT(writeback_queue_io,
 		__entry->age	= older_than_this ?
 				  (jiffies - *older_than_this) * 1000 / HZ : -1;
 		__entry->moved	= moved;
+		__entry->reason	= work->reason;
 	),
-	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d",
+	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d reason=%s",
 		__entry->name,
 		__entry->older,	/* older_than_this in jiffies */
 		__entry->age,	/* older_than_this in relative milliseconds */
-		__entry->moved)
+		__entry->moved,
+		show_work_reason(__entry->reason))
 );
 
 TRACE_EVENT(global_dirty_state,
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index d6edf8d..474bcfe 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -456,7 +456,8 @@ static int bdi_forker_thread(void *ptr)
 				 * the bdi from the thread. Hopefully 1024 is
 				 * large enough for efficient IO.
 				 */
-				writeback_inodes_wb(&bdi->wb, 1024);
+				writeback_inodes_wb(&bdi->wb, 1024,
+						    WB_REASON_FORKER_THREAD);
 			} else {
 				/*
 				 * The spinlock makes sure we do not lose
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index d196074..0e78252 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -738,7 +738,8 @@ static void balance_dirty_pages(struct address_space *mapping,
 		trace_balance_dirty_start(bdi);
 		if (bdi_nr_reclaimable > task_bdi_thresh) {
 			pages_written += writeback_inodes_wb(&bdi->wb,
-							     write_chunk);
+						write_chunk,
+						WB_REASON_BALANCE_DIRTY);
 			trace_balance_dirty_written(bdi, pages_written);
 			if (pages_written >= write_chunk)
 				break;		/* We've done our duty */
@@ -909,7 +910,8 @@ void laptop_mode_timer_fn(unsigned long data)
 	 * threshold
 	 */
 	if (bdi_has_dirty_io(&q->backing_dev_info))
-		bdi_start_writeback(&q->backing_dev_info, nr_pages);
+		bdi_start_writeback(&q->backing_dev_info, nr_pages,
+					WB_REASON_LAPTOP_TIMER);
 }
 
 /*
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7ef6912..3b6c7ac 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -495,7 +495,6 @@ static pageout_t pageout(struct page *page, struct address_space *mapping,
 			ClearPageReclaim(page);
 			return PAGE_ACTIVATE;
 		}
-
 		/*
 		 * Wait on writeback if requested to. This happens when
 		 * direct reclaiming a large contiguous area and the
@@ -2198,7 +2197,8 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 		 */
 		writeback_threshold = sc->nr_to_reclaim + sc->nr_to_reclaim / 2;
 		if (total_scanned > writeback_threshold) {
-			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned);
+			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned,
+						WB_REASON_TRY_TO_FREE_PAGES);
 			sc->may_writepage = 1;
 		}
 
-- 
1.7.3.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 3/3 v3] writeback: Add writeback stats for pages written
  2011-08-22 18:38 ` Curt Wohlgemuth
@ 2011-08-22 18:38   ` Curt Wohlgemuth
  -1 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-22 18:38 UTC (permalink / raw)
  To: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton, Dave Chinner
  Cc: linux-fsdevel, linux-mm, Curt Wohlgemuth

Add a new file, /proc/writeback, which displays
machine global data for how many pages were cleaned for
which reasons.

These data are also available for each BDI, in
<debugfs mount point>/bdi/<device>/wbstats .

Sample output:

   page: balance_dirty_pages               708
   page: background_writeout           3705522
   page: try_to_free_pages                   0
   page: sync                                0
   page: periodic                       269589
   page: fdatawrite                     831528
   page: laptop_periodic                     0
   page: free_more_memory                    0
   page: fs_free_space                       0

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---

Changes since v2:

   - Global stats are now in /proc/writeback , not
     /proc/writeback/stats
   - Per-BDI stats are now in
        <debugfs mount point>/bdi/<device>/wbstats
     not in /sys/block/<device>/bdi/writeback_stats
   - Stats now only include pages written for each reason
   - All files are now non-writeable
             

I didn't address two issues raised by Fengguang from v2 of
this patch:

   - Global data could possibly go into /proc/vmstat, instead of
     into the new /proc/writeback file.
   - The form of the stats could be more useful if they specified
     more than just pages for each reason, but also (a) how many
     'work' items were used for each reason; and (b) how many
     chunks of pages were send for each reason.  E.g.:

                              pages  chunks  works  chunk_kb  work_kbps
       balance_dirty_pages     xx       xx     xx      
       background              xx
       sync

Fengguang, I think this might be useful, but it's a fairly
complex change, that I suspect would be better handled in a
separate patch.  What do you think?


 fs/fs-writeback.c           |   10 +++-
 fs/proc/root.c              |    2 +
 include/linux/backing-dev.h |   10 +++
 include/linux/writeback.h   |    2 +
 mm/backing-dev.c            |  143 ++++++++++++++++++++++++++++++++++++++++++-
 mm/filemap.c                |    4 +
 mm/page-writeback.c         |    7 ++-
 7 files changed, 174 insertions(+), 4 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index a004fcd..dc5ed10 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -542,6 +542,7 @@ static long writeback_sb_inodes(struct super_block *sb,
 
 	while (!list_empty(&wb->b_io)) {
 		struct inode *inode = wb_inode(wb->b_io.prev);
+		long wrote_this_inode;
 
 		if (inode->i_sb != sb) {
 			if (work->sb) {
@@ -580,8 +581,10 @@ static long writeback_sb_inodes(struct super_block *sb,
 
 		writeback_single_inode(inode, wb, &wbc);
 
-		work->nr_pages -= write_chunk - wbc.nr_to_write;
-		wrote += write_chunk - wbc.nr_to_write;
+		wrote_this_inode = write_chunk - wbc.nr_to_write;
+
+		work->nr_pages -= wrote_this_inode;
+		wrote += wrote_this_inode;
 		if (!(inode->i_state & I_DIRTY))
 			wrote++;
 		if (wbc.pages_skipped) {
@@ -591,6 +594,9 @@ static long writeback_sb_inodes(struct super_block *sb,
 			 */
 			redirty_tail(inode, wb);
 		}
+		bdi_writeback_stat_add(wb->bdi,
+					work->reason,
+					wrote_this_inode);
 		spin_unlock(&inode->i_lock);
 		spin_unlock(&wb->list_lock);
 		iput(inode);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 9a8a2b7..c0e2412 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -18,6 +18,7 @@
 #include <linux/bitops.h>
 #include <linux/mount.h>
 #include <linux/pid_namespace.h>
+#include <linux/backing-dev.h>
 
 #include "internal.h"
 
@@ -125,6 +126,7 @@ void __init proc_root_init(void)
 #endif
 	proc_mkdir("bus", NULL);
 	proc_sys_init();
+	proc_writeback_init();
 }
 
 static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index ef85559..8899fec 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -44,6 +44,10 @@ enum bdi_stat_item {
 	NR_BDI_STAT_ITEMS
 };
 
+struct writeback_stats {
+	u64 stats[WB_REASON_MAX];
+};
+
 #define BDI_STAT_BATCH (8*(1+ilog2(nr_cpu_ids)))
 
 struct bdi_writeback {
@@ -72,6 +76,7 @@ struct backing_dev_info {
 	char *name;
 
 	struct percpu_counter bdi_stat[NR_BDI_STAT_ITEMS];
+	struct writeback_stats *wb_stat;
 
 	unsigned long bw_time_stamp;	/* last time write bw is updated */
 	unsigned long written_stamp;	/* pages written at bw_time_stamp */
@@ -96,6 +101,7 @@ struct backing_dev_info {
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debug_dir;
 	struct dentry *debug_stats;
+	struct dentry *debug_wbstats;
 #endif
 };
 
@@ -190,6 +196,10 @@ static inline s64 bdi_stat_sum(struct backing_dev_info *bdi,
 	return sum;
 }
 
+void bdi_writeback_stat_add(struct backing_dev_info *bdi,
+			    enum wb_reason reason, unsigned long value);
+void proc_writeback_init(void);
+
 extern void bdi_writeout_inc(struct backing_dev_info *bdi);
 
 /*
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index bdda069..5168ac9 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -59,6 +59,7 @@ enum wb_reason {
 	WB_REASON_TRY_TO_FREE_PAGES,
 	WB_REASON_SYNC,
 	WB_REASON_PERIODIC,
+	WB_REASON_FDATAWRITE,
 	WB_REASON_LAPTOP_TIMER,
 	WB_REASON_FREE_MORE_MEM,
 	WB_REASON_FS_FREE_SPACE,
@@ -67,6 +68,7 @@ enum wb_reason {
 	WB_REASON_MAX,
 };
 
+
 /*
  * A control structure which tells the writeback code what to do.  These are
  * always on the stack, and hence need no locking.  They are always initialised
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 474bcfe..6613391 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -10,6 +10,8 @@
 #include <linux/module.h>
 #include <linux/writeback.h>
 #include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
 #include <trace/events/writeback.h>
 
 static atomic_long_t bdi_seq = ATOMIC_LONG_INIT(0);
@@ -42,6 +44,8 @@ LIST_HEAD(bdi_pending_list);
 static struct task_struct *sync_supers_tsk;
 static struct timer_list sync_supers_timer;
 
+static struct writeback_stats *writeback_sys_stats;
+
 static int bdi_sync_supers(void *);
 static void sync_supers_timer_fn(unsigned long);
 
@@ -56,9 +60,77 @@ void bdi_lock_two(struct bdi_writeback *wb1, struct bdi_writeback *wb2)
 	}
 }
 
+
+static const char *wb_stats_labels[WB_REASON_MAX] = {
+	[WB_REASON_BALANCE_DIRTY] = "page: balance_dirty_pages",
+	[WB_REASON_BACKGROUND] = "page: background_writeout",
+	[WB_REASON_TRY_TO_FREE_PAGES] = "page: try_to_free_pages",
+	[WB_REASON_SYNC] = "page: sync",
+	[WB_REASON_PERIODIC] = "page: periodic",
+	[WB_REASON_FDATAWRITE] = "page: fdatawrite",
+	[WB_REASON_LAPTOP_TIMER] = "page: laptop_periodic",
+	[WB_REASON_FREE_MORE_MEM] = "page: free_more_memory",
+	[WB_REASON_FS_FREE_SPACE] = "page: fs_free_space",
+};
+
+static void writeback_stats_collect(struct writeback_stats *src,
+			struct writeback_stats *target)
+{
+	int cpu;
+	for_each_online_cpu(cpu) {
+		int stat;
+		struct writeback_stats *stats = per_cpu_ptr(src, cpu);
+		for (stat = 0; stat < WB_REASON_MAX; stat++)
+			target->stats[stat] += stats->stats[stat];
+	}
+}
+
+static size_t writeback_stats_to_str(struct writeback_stats *stats,
+				    char *buf, size_t len)
+{
+	int bufsize = len - 1;
+	int i, printed = 0;
+	for (i = 0; i < WB_REASON_MAX; i++) {
+		const char *label = wb_stats_labels[i];
+		if (label == NULL)
+			continue;
+		printed += snprintf(buf + printed, bufsize - printed,
+				"%-32s %10llu\n", label, stats->stats[i]);
+		if (printed >= bufsize) {
+			buf[len - 1] = '\n';
+			return len;
+		}
+	}
+
+	buf[printed - 1] = '\n';
+	return printed;
+}
+
+static size_t writeback_stats_print(struct writeback_stats *stats,
+				     char *buf, size_t len)
+{
+	struct writeback_stats total;
+	memset(&total, 0, sizeof(total));
+	writeback_stats_collect(stats, &total);
+	return writeback_stats_to_str(&total, buf, len);
+}
+
+static int writeback_seq_show(struct seq_file *m, void *data)
+{
+	char *buf;
+	size_t size;
+	struct writeback_stats *stats = m->private;
+
+	size = seq_get_buf(m, &buf);
+	if (size == 0)
+		return 0;
+	size = writeback_stats_print(stats, buf, size);
+	seq_commit(m, size);
+	return 0;
+}
+
 #ifdef CONFIG_DEBUG_FS
 #include <linux/debugfs.h>
-#include <linux/seq_file.h>
 
 static struct dentry *bdi_debug_root;
 
@@ -132,15 +204,34 @@ static const struct file_operations bdi_debug_stats_fops = {
 	.release	= single_release,
 };
 
+static int bdi_debug_wbstats_open(struct inode *inode, struct file *file)
+{
+	struct backing_dev_info *bdi = inode->i_private;
+	struct writeback_stats *stats = bdi->wb_stat;
+
+	return single_open(file, writeback_seq_show, (void *)stats);
+}
+
+static const struct file_operations bdi_debug_wbstats_fops = {
+	.open		= bdi_debug_wbstats_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static void bdi_debug_register(struct backing_dev_info *bdi, const char *name)
 {
 	bdi->debug_dir = debugfs_create_dir(name, bdi_debug_root);
 	bdi->debug_stats = debugfs_create_file("stats", 0444, bdi->debug_dir,
 					       bdi, &bdi_debug_stats_fops);
+	bdi->debug_wbstats = debugfs_create_file("wbstats", 0444,
+						 bdi->debug_dir, bdi,
+						 &bdi_debug_wbstats_fops);
 }
 
 static void bdi_debug_unregister(struct backing_dev_info *bdi)
 {
+	debugfs_remove(bdi->debug_wbstats);
 	debugfs_remove(bdi->debug_stats);
 	debugfs_remove(bdi->debug_dir);
 }
@@ -157,6 +248,7 @@ static inline void bdi_debug_unregister(struct backing_dev_info *bdi)
 }
 #endif
 
+
 static ssize_t read_ahead_kb_store(struct device *dev,
 				  struct device_attribute *attr,
 				  const char *buf, size_t count)
@@ -678,8 +770,13 @@ int bdi_init(struct backing_dev_info *bdi)
 
 	err = prop_local_init_percpu(&bdi->completions);
 
+	bdi->wb_stat = alloc_percpu(struct writeback_stats);
+	if (bdi->wb_stat == NULL)
+		err = -ENOMEM;
+
 	if (err) {
 err:
+		free_percpu(bdi->wb_stat);
 		while (i--)
 			percpu_counter_destroy(&bdi->bdi_stat[i]);
 	}
@@ -712,6 +809,8 @@ void bdi_destroy(struct backing_dev_info *bdi)
 	for (i = 0; i < NR_BDI_STAT_ITEMS; i++)
 		percpu_counter_destroy(&bdi->bdi_stat[i]);
 
+	free_percpu(bdi->wb_stat);
+
 	prop_local_destroy_percpu(&bdi->completions);
 }
 EXPORT_SYMBOL(bdi_destroy);
@@ -854,3 +953,45 @@ out:
 	return ret;
 }
 EXPORT_SYMBOL(wait_iff_congested);
+
+void bdi_writeback_stat_add(struct backing_dev_info *bdi,
+			    enum wb_reason reason, unsigned long value)
+{
+	if (bdi) {
+		struct writeback_stats *stats = bdi->wb_stat;
+
+		BUG_ON(reason >= WB_REASON_MAX);
+		preempt_disable();
+		stats = per_cpu_ptr(stats, smp_processor_id());
+		stats->stats[reason] += value;
+		if (likely(writeback_sys_stats)) {
+			stats = per_cpu_ptr(writeback_sys_stats,
+					    smp_processor_id());
+			stats->stats[reason] += value;
+		}
+		preempt_enable();
+	}
+}
+
+
+static int global_writeback_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, writeback_seq_show,
+					(void *)writeback_sys_stats);
+}
+
+static const struct file_operations global_writeback_ops = {
+	.open           = global_writeback_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = single_release,
+};
+
+
+void __init proc_writeback_init(void)
+{
+	writeback_sys_stats = alloc_percpu(struct writeback_stats);
+
+	proc_create_data("writeback", S_IRUGO, NULL,
+			&global_writeback_ops, NULL);
+}
diff --git a/mm/filemap.c b/mm/filemap.c
index 645a080..cc93a9c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -216,6 +216,10 @@ int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
 		return 0;
 
 	ret = do_writepages(mapping, &wbc);
+
+	bdi_writeback_stat_add(mapping->backing_dev_info,
+				WB_REASON_FDATAWRITE,
+				LONG_MAX - wbc.nr_to_write);
 	return ret;
 }
 
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0e78252..36bc09b 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -737,9 +737,14 @@ static void balance_dirty_pages(struct address_space *mapping,
 		 */
 		trace_balance_dirty_start(bdi);
 		if (bdi_nr_reclaimable > task_bdi_thresh) {
-			pages_written += writeback_inodes_wb(&bdi->wb,
+			long wrote;
+			wrote = writeback_inodes_wb(&bdi->wb,
 						write_chunk,
 						WB_REASON_BALANCE_DIRTY);
+			pages_written += wrote;
+			bdi_writeback_stat_add(bdi,
+						WB_REASON_BALANCE_DIRTY,
+						wrote);
 			trace_balance_dirty_written(bdi, pages_written);
 			if (pages_written >= write_chunk)
 				break;		/* We've done our duty */
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 3/3 v3] writeback: Add writeback stats for pages written
@ 2011-08-22 18:38   ` Curt Wohlgemuth
  0 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-22 18:38 UTC (permalink / raw)
  To: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin
  Cc: linux-fsdevel, linux-mm, Curt Wohlgemuth

Add a new file, /proc/writeback, which displays
machine global data for how many pages were cleaned for
which reasons.

These data are also available for each BDI, in
<debugfs mount point>/bdi/<device>/wbstats .

Sample output:

   page: balance_dirty_pages               708
   page: background_writeout           3705522
   page: try_to_free_pages                   0
   page: sync                                0
   page: periodic                       269589
   page: fdatawrite                     831528
   page: laptop_periodic                     0
   page: free_more_memory                    0
   page: fs_free_space                       0

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---

Changes since v2:

   - Global stats are now in /proc/writeback , not
     /proc/writeback/stats
   - Per-BDI stats are now in
        <debugfs mount point>/bdi/<device>/wbstats
     not in /sys/block/<device>/bdi/writeback_stats
   - Stats now only include pages written for each reason
   - All files are now non-writeable
             

I didn't address two issues raised by Fengguang from v2 of
this patch:

   - Global data could possibly go into /proc/vmstat, instead of
     into the new /proc/writeback file.
   - The form of the stats could be more useful if they specified
     more than just pages for each reason, but also (a) how many
     'work' items were used for each reason; and (b) how many
     chunks of pages were send for each reason.  E.g.:

                              pages  chunks  works  chunk_kb  work_kbps
       balance_dirty_pages     xx       xx     xx      
       background              xx
       sync

Fengguang, I think this might be useful, but it's a fairly
complex change, that I suspect would be better handled in a
separate patch.  What do you think?


 fs/fs-writeback.c           |   10 +++-
 fs/proc/root.c              |    2 +
 include/linux/backing-dev.h |   10 +++
 include/linux/writeback.h   |    2 +
 mm/backing-dev.c            |  143 ++++++++++++++++++++++++++++++++++++++++++-
 mm/filemap.c                |    4 +
 mm/page-writeback.c         |    7 ++-
 7 files changed, 174 insertions(+), 4 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index a004fcd..dc5ed10 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -542,6 +542,7 @@ static long writeback_sb_inodes(struct super_block *sb,
 
 	while (!list_empty(&wb->b_io)) {
 		struct inode *inode = wb_inode(wb->b_io.prev);
+		long wrote_this_inode;
 
 		if (inode->i_sb != sb) {
 			if (work->sb) {
@@ -580,8 +581,10 @@ static long writeback_sb_inodes(struct super_block *sb,
 
 		writeback_single_inode(inode, wb, &wbc);
 
-		work->nr_pages -= write_chunk - wbc.nr_to_write;
-		wrote += write_chunk - wbc.nr_to_write;
+		wrote_this_inode = write_chunk - wbc.nr_to_write;
+
+		work->nr_pages -= wrote_this_inode;
+		wrote += wrote_this_inode;
 		if (!(inode->i_state & I_DIRTY))
 			wrote++;
 		if (wbc.pages_skipped) {
@@ -591,6 +594,9 @@ static long writeback_sb_inodes(struct super_block *sb,
 			 */
 			redirty_tail(inode, wb);
 		}
+		bdi_writeback_stat_add(wb->bdi,
+					work->reason,
+					wrote_this_inode);
 		spin_unlock(&inode->i_lock);
 		spin_unlock(&wb->list_lock);
 		iput(inode);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 9a8a2b7..c0e2412 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -18,6 +18,7 @@
 #include <linux/bitops.h>
 #include <linux/mount.h>
 #include <linux/pid_namespace.h>
+#include <linux/backing-dev.h>
 
 #include "internal.h"
 
@@ -125,6 +126,7 @@ void __init proc_root_init(void)
 #endif
 	proc_mkdir("bus", NULL);
 	proc_sys_init();
+	proc_writeback_init();
 }
 
 static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index ef85559..8899fec 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -44,6 +44,10 @@ enum bdi_stat_item {
 	NR_BDI_STAT_ITEMS
 };
 
+struct writeback_stats {
+	u64 stats[WB_REASON_MAX];
+};
+
 #define BDI_STAT_BATCH (8*(1+ilog2(nr_cpu_ids)))
 
 struct bdi_writeback {
@@ -72,6 +76,7 @@ struct backing_dev_info {
 	char *name;
 
 	struct percpu_counter bdi_stat[NR_BDI_STAT_ITEMS];
+	struct writeback_stats *wb_stat;
 
 	unsigned long bw_time_stamp;	/* last time write bw is updated */
 	unsigned long written_stamp;	/* pages written at bw_time_stamp */
@@ -96,6 +101,7 @@ struct backing_dev_info {
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debug_dir;
 	struct dentry *debug_stats;
+	struct dentry *debug_wbstats;
 #endif
 };
 
@@ -190,6 +196,10 @@ static inline s64 bdi_stat_sum(struct backing_dev_info *bdi,
 	return sum;
 }
 
+void bdi_writeback_stat_add(struct backing_dev_info *bdi,
+			    enum wb_reason reason, unsigned long value);
+void proc_writeback_init(void);
+
 extern void bdi_writeout_inc(struct backing_dev_info *bdi);
 
 /*
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index bdda069..5168ac9 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -59,6 +59,7 @@ enum wb_reason {
 	WB_REASON_TRY_TO_FREE_PAGES,
 	WB_REASON_SYNC,
 	WB_REASON_PERIODIC,
+	WB_REASON_FDATAWRITE,
 	WB_REASON_LAPTOP_TIMER,
 	WB_REASON_FREE_MORE_MEM,
 	WB_REASON_FS_FREE_SPACE,
@@ -67,6 +68,7 @@ enum wb_reason {
 	WB_REASON_MAX,
 };
 
+
 /*
  * A control structure which tells the writeback code what to do.  These are
  * always on the stack, and hence need no locking.  They are always initialised
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 474bcfe..6613391 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -10,6 +10,8 @@
 #include <linux/module.h>
 #include <linux/writeback.h>
 #include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
 #include <trace/events/writeback.h>
 
 static atomic_long_t bdi_seq = ATOMIC_LONG_INIT(0);
@@ -42,6 +44,8 @@ LIST_HEAD(bdi_pending_list);
 static struct task_struct *sync_supers_tsk;
 static struct timer_list sync_supers_timer;
 
+static struct writeback_stats *writeback_sys_stats;
+
 static int bdi_sync_supers(void *);
 static void sync_supers_timer_fn(unsigned long);
 
@@ -56,9 +60,77 @@ void bdi_lock_two(struct bdi_writeback *wb1, struct bdi_writeback *wb2)
 	}
 }
 
+
+static const char *wb_stats_labels[WB_REASON_MAX] = {
+	[WB_REASON_BALANCE_DIRTY] = "page: balance_dirty_pages",
+	[WB_REASON_BACKGROUND] = "page: background_writeout",
+	[WB_REASON_TRY_TO_FREE_PAGES] = "page: try_to_free_pages",
+	[WB_REASON_SYNC] = "page: sync",
+	[WB_REASON_PERIODIC] = "page: periodic",
+	[WB_REASON_FDATAWRITE] = "page: fdatawrite",
+	[WB_REASON_LAPTOP_TIMER] = "page: laptop_periodic",
+	[WB_REASON_FREE_MORE_MEM] = "page: free_more_memory",
+	[WB_REASON_FS_FREE_SPACE] = "page: fs_free_space",
+};
+
+static void writeback_stats_collect(struct writeback_stats *src,
+			struct writeback_stats *target)
+{
+	int cpu;
+	for_each_online_cpu(cpu) {
+		int stat;
+		struct writeback_stats *stats = per_cpu_ptr(src, cpu);
+		for (stat = 0; stat < WB_REASON_MAX; stat++)
+			target->stats[stat] += stats->stats[stat];
+	}
+}
+
+static size_t writeback_stats_to_str(struct writeback_stats *stats,
+				    char *buf, size_t len)
+{
+	int bufsize = len - 1;
+	int i, printed = 0;
+	for (i = 0; i < WB_REASON_MAX; i++) {
+		const char *label = wb_stats_labels[i];
+		if (label == NULL)
+			continue;
+		printed += snprintf(buf + printed, bufsize - printed,
+				"%-32s %10llu\n", label, stats->stats[i]);
+		if (printed >= bufsize) {
+			buf[len - 1] = '\n';
+			return len;
+		}
+	}
+
+	buf[printed - 1] = '\n';
+	return printed;
+}
+
+static size_t writeback_stats_print(struct writeback_stats *stats,
+				     char *buf, size_t len)
+{
+	struct writeback_stats total;
+	memset(&total, 0, sizeof(total));
+	writeback_stats_collect(stats, &total);
+	return writeback_stats_to_str(&total, buf, len);
+}
+
+static int writeback_seq_show(struct seq_file *m, void *data)
+{
+	char *buf;
+	size_t size;
+	struct writeback_stats *stats = m->private;
+
+	size = seq_get_buf(m, &buf);
+	if (size == 0)
+		return 0;
+	size = writeback_stats_print(stats, buf, size);
+	seq_commit(m, size);
+	return 0;
+}
+
 #ifdef CONFIG_DEBUG_FS
 #include <linux/debugfs.h>
-#include <linux/seq_file.h>
 
 static struct dentry *bdi_debug_root;
 
@@ -132,15 +204,34 @@ static const struct file_operations bdi_debug_stats_fops = {
 	.release	= single_release,
 };
 
+static int bdi_debug_wbstats_open(struct inode *inode, struct file *file)
+{
+	struct backing_dev_info *bdi = inode->i_private;
+	struct writeback_stats *stats = bdi->wb_stat;
+
+	return single_open(file, writeback_seq_show, (void *)stats);
+}
+
+static const struct file_operations bdi_debug_wbstats_fops = {
+	.open		= bdi_debug_wbstats_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static void bdi_debug_register(struct backing_dev_info *bdi, const char *name)
 {
 	bdi->debug_dir = debugfs_create_dir(name, bdi_debug_root);
 	bdi->debug_stats = debugfs_create_file("stats", 0444, bdi->debug_dir,
 					       bdi, &bdi_debug_stats_fops);
+	bdi->debug_wbstats = debugfs_create_file("wbstats", 0444,
+						 bdi->debug_dir, bdi,
+						 &bdi_debug_wbstats_fops);
 }
 
 static void bdi_debug_unregister(struct backing_dev_info *bdi)
 {
+	debugfs_remove(bdi->debug_wbstats);
 	debugfs_remove(bdi->debug_stats);
 	debugfs_remove(bdi->debug_dir);
 }
@@ -157,6 +248,7 @@ static inline void bdi_debug_unregister(struct backing_dev_info *bdi)
 }
 #endif
 
+
 static ssize_t read_ahead_kb_store(struct device *dev,
 				  struct device_attribute *attr,
 				  const char *buf, size_t count)
@@ -678,8 +770,13 @@ int bdi_init(struct backing_dev_info *bdi)
 
 	err = prop_local_init_percpu(&bdi->completions);
 
+	bdi->wb_stat = alloc_percpu(struct writeback_stats);
+	if (bdi->wb_stat == NULL)
+		err = -ENOMEM;
+
 	if (err) {
 err:
+		free_percpu(bdi->wb_stat);
 		while (i--)
 			percpu_counter_destroy(&bdi->bdi_stat[i]);
 	}
@@ -712,6 +809,8 @@ void bdi_destroy(struct backing_dev_info *bdi)
 	for (i = 0; i < NR_BDI_STAT_ITEMS; i++)
 		percpu_counter_destroy(&bdi->bdi_stat[i]);
 
+	free_percpu(bdi->wb_stat);
+
 	prop_local_destroy_percpu(&bdi->completions);
 }
 EXPORT_SYMBOL(bdi_destroy);
@@ -854,3 +953,45 @@ out:
 	return ret;
 }
 EXPORT_SYMBOL(wait_iff_congested);
+
+void bdi_writeback_stat_add(struct backing_dev_info *bdi,
+			    enum wb_reason reason, unsigned long value)
+{
+	if (bdi) {
+		struct writeback_stats *stats = bdi->wb_stat;
+
+		BUG_ON(reason >= WB_REASON_MAX);
+		preempt_disable();
+		stats = per_cpu_ptr(stats, smp_processor_id());
+		stats->stats[reason] += value;
+		if (likely(writeback_sys_stats)) {
+			stats = per_cpu_ptr(writeback_sys_stats,
+					    smp_processor_id());
+			stats->stats[reason] += value;
+		}
+		preempt_enable();
+	}
+}
+
+
+static int global_writeback_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, writeback_seq_show,
+					(void *)writeback_sys_stats);
+}
+
+static const struct file_operations global_writeback_ops = {
+	.open           = global_writeback_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = single_release,
+};
+
+
+void __init proc_writeback_init(void)
+{
+	writeback_sys_stats = alloc_percpu(struct writeback_stats);
+
+	proc_create_data("writeback", S_IRUGO, NULL,
+			&global_writeback_ops, NULL);
+}
diff --git a/mm/filemap.c b/mm/filemap.c
index 645a080..cc93a9c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -216,6 +216,10 @@ int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
 		return 0;
 
 	ret = do_writepages(mapping, &wbc);
+
+	bdi_writeback_stat_add(mapping->backing_dev_info,
+				WB_REASON_FDATAWRITE,
+				LONG_MAX - wbc.nr_to_write);
 	return ret;
 }
 
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0e78252..36bc09b 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -737,9 +737,14 @@ static void balance_dirty_pages(struct address_space *mapping,
 		 */
 		trace_balance_dirty_start(bdi);
 		if (bdi_nr_reclaimable > task_bdi_thresh) {
-			pages_written += writeback_inodes_wb(&bdi->wb,
+			long wrote;
+			wrote = writeback_inodes_wb(&bdi->wb,
 						write_chunk,
 						WB_REASON_BALANCE_DIRTY);
+			pages_written += wrote;
+			bdi_writeback_stat_add(bdi,
+						WB_REASON_BALANCE_DIRTY,
+						wrote);
 			trace_balance_dirty_written(bdi, pages_written);
 			if (pages_written >= write_chunk)
 				break;		/* We've done our duty */
-- 
1.7.3.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] writeback: send work item to queue_io, move_expired_inodes
  2011-08-22 18:38 ` Curt Wohlgemuth
@ 2011-08-29 16:21   ` Jan Kara
  -1 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-29 16:21 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Mon 22-08-11 11:38:45, Curt Wohlgemuth wrote:
> Instead of sending ->older_than_this to queue_io() and
> move_expired_inodes(), send the entire wb_writeback_work
> structure.  There are other fields of a work item that are
> useful in these routines and in tracepoints.
  Looks good. You can add: Acked-by: Jan Kara <jack@suse.cz>

> Signed-off-by: Curt Wohlgemuth <curtw@google.com>
> ---
>  fs/fs-writeback.c                |   16 ++++++++--------
>  include/trace/events/writeback.h |    5 +++--
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 04cf3b9..f1f5f65 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -251,7 +251,7 @@ static bool inode_dirtied_after(struct inode *inode, unsigned long t)
>   */
>  static int move_expired_inodes(struct list_head *delaying_queue,
>  			       struct list_head *dispatch_queue,
> -			       unsigned long *older_than_this)
> +			       struct wb_writeback_work *work)
>  {
>  	LIST_HEAD(tmp);
>  	struct list_head *pos, *node;
> @@ -262,8 +262,8 @@ static int move_expired_inodes(struct list_head *delaying_queue,
>  
>  	while (!list_empty(delaying_queue)) {
>  		inode = wb_inode(delaying_queue->prev);
> -		if (older_than_this &&
> -		    inode_dirtied_after(inode, *older_than_this))
> +		if (work->older_than_this &&
> +		    inode_dirtied_after(inode, *work->older_than_this))
>  			break;
>  		if (sb && sb != inode->i_sb)
>  			do_sb_sort = 1;
> @@ -302,13 +302,13 @@ out:
>   *                                           |
>   *                                           +--> dequeue for IO
>   */
> -static void queue_io(struct bdi_writeback *wb, unsigned long *older_than_this)
> +static void queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work)
>  {
>  	int moved;
>  	assert_spin_locked(&wb->list_lock);
>  	list_splice_init(&wb->b_more_io, &wb->b_io);
> -	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, older_than_this);
> -	trace_writeback_queue_io(wb, older_than_this, moved);
> +	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, work);
> +	trace_writeback_queue_io(wb, work, moved);
>  }
>  
>  static int write_inode(struct inode *inode, struct writeback_control *wbc)
> @@ -651,7 +651,7 @@ long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
>  
>  	spin_lock(&wb->list_lock);
>  	if (list_empty(&wb->b_io))
> -		queue_io(wb, NULL);
> +		queue_io(wb, &work);
>  	__writeback_inodes_wb(wb, &work);
>  	spin_unlock(&wb->list_lock);
>  
> @@ -738,7 +738,7 @@ static long wb_writeback(struct bdi_writeback *wb,
>  
>  		trace_writeback_start(wb->bdi, work);
>  		if (list_empty(&wb->b_io))
> -			queue_io(wb, work->older_than_this);
> +			queue_io(wb, work);
>  		if (work->sb)
>  			progress = writeback_sb_inodes(work->sb, wb, work);
>  		else
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index 6bca4cc..4565274 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -181,9 +181,9 @@ DEFINE_WBC_EVENT(wbc_writepage);
>  
>  TRACE_EVENT(writeback_queue_io,
>  	TP_PROTO(struct bdi_writeback *wb,
> -		 unsigned long *older_than_this,
> +		 struct wb_writeback_work *work,
>  		 int moved),
> -	TP_ARGS(wb, older_than_this, moved),
> +	TP_ARGS(wb, work, moved),
>  	TP_STRUCT__entry(
>  		__array(char,		name, 32)
>  		__field(unsigned long,	older)
> @@ -191,6 +191,7 @@ TRACE_EVENT(writeback_queue_io,
>  		__field(int,		moved)
>  	),
>  	TP_fast_assign(
> +		unsigned long *older_than_this = work->older_than_this;
>  		strncpy(__entry->name, dev_name(wb->bdi->dev), 32);
>  		__entry->older	= older_than_this ?  *older_than_this : 0;
>  		__entry->age	= older_than_this ?
> -- 
> 1.7.3.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] writeback: send work item to queue_io, move_expired_inodes
@ 2011-08-29 16:21   ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-29 16:21 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Mon 22-08-11 11:38:45, Curt Wohlgemuth wrote:
> Instead of sending ->older_than_this to queue_io() and
> move_expired_inodes(), send the entire wb_writeback_work
> structure.  There are other fields of a work item that are
> useful in these routines and in tracepoints.
  Looks good. You can add: Acked-by: Jan Kara <jack@suse.cz>

> Signed-off-by: Curt Wohlgemuth <curtw@google.com>
> ---
>  fs/fs-writeback.c                |   16 ++++++++--------
>  include/trace/events/writeback.h |    5 +++--
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 04cf3b9..f1f5f65 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -251,7 +251,7 @@ static bool inode_dirtied_after(struct inode *inode, unsigned long t)
>   */
>  static int move_expired_inodes(struct list_head *delaying_queue,
>  			       struct list_head *dispatch_queue,
> -			       unsigned long *older_than_this)
> +			       struct wb_writeback_work *work)
>  {
>  	LIST_HEAD(tmp);
>  	struct list_head *pos, *node;
> @@ -262,8 +262,8 @@ static int move_expired_inodes(struct list_head *delaying_queue,
>  
>  	while (!list_empty(delaying_queue)) {
>  		inode = wb_inode(delaying_queue->prev);
> -		if (older_than_this &&
> -		    inode_dirtied_after(inode, *older_than_this))
> +		if (work->older_than_this &&
> +		    inode_dirtied_after(inode, *work->older_than_this))
>  			break;
>  		if (sb && sb != inode->i_sb)
>  			do_sb_sort = 1;
> @@ -302,13 +302,13 @@ out:
>   *                                           |
>   *                                           +--> dequeue for IO
>   */
> -static void queue_io(struct bdi_writeback *wb, unsigned long *older_than_this)
> +static void queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work)
>  {
>  	int moved;
>  	assert_spin_locked(&wb->list_lock);
>  	list_splice_init(&wb->b_more_io, &wb->b_io);
> -	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, older_than_this);
> -	trace_writeback_queue_io(wb, older_than_this, moved);
> +	moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, work);
> +	trace_writeback_queue_io(wb, work, moved);
>  }
>  
>  static int write_inode(struct inode *inode, struct writeback_control *wbc)
> @@ -651,7 +651,7 @@ long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
>  
>  	spin_lock(&wb->list_lock);
>  	if (list_empty(&wb->b_io))
> -		queue_io(wb, NULL);
> +		queue_io(wb, &work);
>  	__writeback_inodes_wb(wb, &work);
>  	spin_unlock(&wb->list_lock);
>  
> @@ -738,7 +738,7 @@ static long wb_writeback(struct bdi_writeback *wb,
>  
>  		trace_writeback_start(wb->bdi, work);
>  		if (list_empty(&wb->b_io))
> -			queue_io(wb, work->older_than_this);
> +			queue_io(wb, work);
>  		if (work->sb)
>  			progress = writeback_sb_inodes(work->sb, wb, work);
>  		else
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index 6bca4cc..4565274 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -181,9 +181,9 @@ DEFINE_WBC_EVENT(wbc_writepage);
>  
>  TRACE_EVENT(writeback_queue_io,
>  	TP_PROTO(struct bdi_writeback *wb,
> -		 unsigned long *older_than_this,
> +		 struct wb_writeback_work *work,
>  		 int moved),
> -	TP_ARGS(wb, older_than_this, moved),
> +	TP_ARGS(wb, work, moved),
>  	TP_STRUCT__entry(
>  		__array(char,		name, 32)
>  		__field(unsigned long,	older)
> @@ -191,6 +191,7 @@ TRACE_EVENT(writeback_queue_io,
>  		__field(int,		moved)
>  	),
>  	TP_fast_assign(
> +		unsigned long *older_than_this = work->older_than_this;
>  		strncpy(__entry->name, dev_name(wb->bdi->dev), 32);
>  		__entry->older	= older_than_this ?  *older_than_this : 0;
>  		__entry->age	= older_than_this ?
> -- 
> 1.7.3.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work
  2011-08-22 18:38   ` Curt Wohlgemuth
@ 2011-08-29 16:23     ` Jan Kara
  -1 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-29 16:23 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Mon 22-08-11 11:38:46, Curt Wohlgemuth wrote:
> This creates a new 'reason' field in a wb_writeback_work
> structure, which unambiguously identifies who initiates
> writeback activity.  A 'wb_reason' enumeration has been
> added to writeback.h, to enumerate the possible reasons.
> 
> The 'writeback_work_class' and tracepoint event class and
> 'writeback_queue_io' tracepoints are updated to include the
> symbolic 'reason' in all trace events.
> 
> And the 'writeback_inodes_sbXXX' family of routines has had
> a wb_stats parameter added to them, so callers can specify
> why writeback is being started.
  Looks good. You can add: Acked-by: Jan Kara <jack@suse.cz>

							Honza
> 
> Signed-off-by: Curt Wohlgemuth <curtw@google.com>
> ---
> 
> Changes since v2:
> 
>    - enum wb_stats renamed enum wb_reason
>    - All WB_STAT_xxx enum constants now named WB_REASON_xxx
>    - All 'reason' strings in tracepoints match the WB_REASON name
>    - The change to send 'work' to queue_io, move_expired_inodes, and
>      trace_writeback_queue_io are in a separate commit
> 
> 
>  fs/btrfs/extent-tree.c           |    3 ++-
>  fs/buffer.c                      |    2 +-
>  fs/ext4/inode.c                  |    2 +-
>  fs/fs-writeback.c                |   38 +++++++++++++++++++++++++-------------
>  fs/quota/quota.c                 |    2 +-
>  fs/sync.c                        |    4 ++--
>  fs/ubifs/budget.c                |    2 +-
>  include/linux/backing-dev.h      |    3 ++-
>  include/linux/writeback.h        |   33 +++++++++++++++++++++++++++------
>  include/trace/events/writeback.h |   27 +++++++++++++++++++++++----
>  mm/backing-dev.c                 |    3 ++-
>  mm/page-writeback.c              |    6 ++++--
>  mm/vmscan.c                      |    4 ++--
>  13 files changed, 93 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index f5be06a..c9ee0e1 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3340,7 +3340,8 @@ static int shrink_delalloc(struct btrfs_trans_handle *trans,
>  		smp_mb();
>  		nr_pages = min_t(unsigned long, nr_pages,
>  		       root->fs_info->delalloc_bytes >> PAGE_CACHE_SHIFT);
> -		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages);
> +		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages,
> +						WB_REASON_FS_FREE_SPACE);
>  
>  		spin_lock(&space_info->lock);
>  		if (reserved > space_info->bytes_reserved)
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 1a80b04..f5dcee6 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -285,7 +285,7 @@ static void free_more_memory(void)
>  	struct zone *zone;
>  	int nid;
>  
> -	wakeup_flusher_threads(1024);
> +	wakeup_flusher_threads(1024, WB_REASON_FREE_MORE_MEM);
>  	yield();
>  
>  	for_each_online_node(nid) {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c4da98a..f4f0f40 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2244,7 +2244,7 @@ static int ext4_nonda_switch(struct super_block *sb)
>  	 * start pushing delalloc when 1/2 of free blocks are dirty.
>  	 */
>  	if (free_blocks < 2 * dirty_blocks)
> -		writeback_inodes_sb_if_idle(sb);
> +		writeback_inodes_sb_if_idle(sb, WB_REASON_FS_FREE_SPACE);
>  
>  	return 0;
>  }
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index f1f5f65..a004fcd 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -41,6 +41,7 @@ struct wb_writeback_work {
>  	unsigned int for_kupdate:1;
>  	unsigned int range_cyclic:1;
>  	unsigned int for_background:1;
> +	enum wb_reason reason;		/* why was writeback initiated? */
>  
>  	struct list_head list;		/* pending work list */
>  	struct completion *done;	/* set if the caller waits */
> @@ -115,7 +116,7 @@ static void bdi_queue_work(struct backing_dev_info *bdi,
>  
>  static void
>  __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
> -		      bool range_cyclic)
> +		      bool range_cyclic, enum wb_reason reason)
>  {
>  	struct wb_writeback_work *work;
>  
> @@ -135,6 +136,7 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
>  	work->sync_mode	= WB_SYNC_NONE;
>  	work->nr_pages	= nr_pages;
>  	work->range_cyclic = range_cyclic;
> +	work->reason = reason;
>  
>  	bdi_queue_work(bdi, work);
>  }
> @@ -150,9 +152,10 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
>   *   completion. Caller need not hold sb s_umount semaphore.
>   *
>   */
> -void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages)
> +void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
> +			enum wb_reason reason)
>  {
> -	__bdi_start_writeback(bdi, nr_pages, true);
> +	__bdi_start_writeback(bdi, nr_pages, true, reason);
>  }
>  
>  /**
> @@ -641,12 +644,14 @@ static long __writeback_inodes_wb(struct bdi_writeback *wb,
>  	return wrote;
>  }
>  
> -long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
> +long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
> +				enum wb_reason reason)
>  {
>  	struct wb_writeback_work work = {
>  		.nr_pages	= nr_pages,
>  		.sync_mode	= WB_SYNC_NONE,
>  		.range_cyclic	= 1,
> +		.reason		= reason,
>  	};
>  
>  	spin_lock(&wb->list_lock);
> @@ -818,6 +823,7 @@ static long wb_check_background_flush(struct bdi_writeback *wb)
>  			.sync_mode	= WB_SYNC_NONE,
>  			.for_background	= 1,
>  			.range_cyclic	= 1,
> +			.reason		= WB_REASON_BACKGROUND,
>  		};
>  
>  		return wb_writeback(wb, &work);
> @@ -851,6 +857,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
>  			.sync_mode	= WB_SYNC_NONE,
>  			.for_kupdate	= 1,
>  			.range_cyclic	= 1,
> +			.reason		= WB_REASON_PERIODIC,
>  		};
>  
>  		return wb_writeback(wb, &work);
> @@ -969,7 +976,7 @@ int bdi_writeback_thread(void *data)
>   * Start writeback of `nr_pages' pages.  If `nr_pages' is zero, write back
>   * the whole world.
>   */
> -void wakeup_flusher_threads(long nr_pages)
> +void wakeup_flusher_threads(long nr_pages, enum wb_reason reason)
>  {
>  	struct backing_dev_info *bdi;
>  
> @@ -982,7 +989,7 @@ void wakeup_flusher_threads(long nr_pages)
>  	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) {
>  		if (!bdi_has_dirty_io(bdi))
>  			continue;
> -		__bdi_start_writeback(bdi, nr_pages, false);
> +		__bdi_start_writeback(bdi, nr_pages, false, reason);
>  	}
>  	rcu_read_unlock();
>  }
> @@ -1203,7 +1210,9 @@ static void wait_sb_inodes(struct super_block *sb)
>   * on how many (if any) will be written, and this function does not wait
>   * for IO completion of submitted IO.
>   */
> -void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
> +void writeback_inodes_sb_nr(struct super_block *sb,
> +			    unsigned long nr,
> +			    enum wb_reason reason)
>  {
>  	DECLARE_COMPLETION_ONSTACK(done);
>  	struct wb_writeback_work work = {
> @@ -1212,6 +1221,7 @@ void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
>  		.tagged_writepages	= 1,
>  		.done			= &done,
>  		.nr_pages		= nr,
> +		.reason			= reason,
>  	};
>  
>  	WARN_ON(!rwsem_is_locked(&sb->s_umount));
> @@ -1228,9 +1238,9 @@ EXPORT_SYMBOL(writeback_inodes_sb_nr);
>   * on how many (if any) will be written, and this function does not wait
>   * for IO completion of submitted IO.
>   */
> -void writeback_inodes_sb(struct super_block *sb)
> +void writeback_inodes_sb(struct super_block *sb, enum wb_reason reason)
>  {
> -	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages());
> +	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages(), reason);
>  }
>  EXPORT_SYMBOL(writeback_inodes_sb);
>  
> @@ -1241,11 +1251,11 @@ EXPORT_SYMBOL(writeback_inodes_sb);
>   * Invoke writeback_inodes_sb if no writeback is currently underway.
>   * Returns 1 if writeback was started, 0 if not.
>   */
> -int writeback_inodes_sb_if_idle(struct super_block *sb)
> +int writeback_inodes_sb_if_idle(struct super_block *sb, enum wb_reason reason)
>  {
>  	if (!writeback_in_progress(sb->s_bdi)) {
>  		down_read(&sb->s_umount);
> -		writeback_inodes_sb(sb);
> +		writeback_inodes_sb(sb, reason);
>  		up_read(&sb->s_umount);
>  		return 1;
>  	} else
> @@ -1262,11 +1272,12 @@ EXPORT_SYMBOL(writeback_inodes_sb_if_idle);
>   * Returns 1 if writeback was started, 0 if not.
>   */
>  int writeback_inodes_sb_nr_if_idle(struct super_block *sb,
> -				   unsigned long nr)
> +				   unsigned long nr,
> +				   enum wb_reason reason)
>  {
>  	if (!writeback_in_progress(sb->s_bdi)) {
>  		down_read(&sb->s_umount);
> -		writeback_inodes_sb_nr(sb, nr);
> +		writeback_inodes_sb_nr(sb, nr, reason);
>  		up_read(&sb->s_umount);
>  		return 1;
>  	} else
> @@ -1290,6 +1301,7 @@ void sync_inodes_sb(struct super_block *sb)
>  		.nr_pages	= LONG_MAX,
>  		.range_cyclic	= 0,
>  		.done		= &done,
> +		.reason		= WB_REASON_SYNC,
>  	};
>  
>  	WARN_ON(!rwsem_is_locked(&sb->s_umount));
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index b34bdb2..b7a4366 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -286,7 +286,7 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
>  		/* caller already holds s_umount */
>  		if (sb->s_flags & MS_RDONLY)
>  			return -EROFS;
> -		writeback_inodes_sb(sb);
> +		writeback_inodes_sb(sb, WB_REASON_SYNC);
>  		return 0;
>  	default:
>  		return -EINVAL;
> diff --git a/fs/sync.c b/fs/sync.c
> index c98a747..101b8ef 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -43,7 +43,7 @@ static int __sync_filesystem(struct super_block *sb, int wait)
>  	if (wait)
>  		sync_inodes_sb(sb);
>  	else
> -		writeback_inodes_sb(sb);
> +		writeback_inodes_sb(sb, WB_REASON_SYNC);
>  
>  	if (sb->s_op->sync_fs)
>  		sb->s_op->sync_fs(sb, wait);
> @@ -98,7 +98,7 @@ static void sync_filesystems(int wait)
>   */
>  SYSCALL_DEFINE0(sync)
>  {
> -	wakeup_flusher_threads(0);
> +	wakeup_flusher_threads(0, WB_REASON_SYNC);
>  	sync_filesystems(0);
>  	sync_filesystems(1);
>  	if (unlikely(laptop_mode))
> diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
> index 315de66..bc4f94b 100644
> --- a/fs/ubifs/budget.c
> +++ b/fs/ubifs/budget.c
> @@ -63,7 +63,7 @@
>  static void shrink_liability(struct ubifs_info *c, int nr_to_write)
>  {
>  	down_read(&c->vfs_sb->s_umount);
> -	writeback_inodes_sb(c->vfs_sb);
> +	writeback_inodes_sb(c->vfs_sb, WB_REASON_FS_FREE_SPACE);
>  	up_read(&c->vfs_sb->s_umount);
>  }
>  
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index 3b2f9cb..ef85559 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -107,7 +107,8 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent,
>  int bdi_register_dev(struct backing_dev_info *bdi, dev_t dev);
>  void bdi_unregister(struct backing_dev_info *bdi);
>  int bdi_setup_and_register(struct backing_dev_info *, char *, unsigned int);
> -void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages);
> +void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
> +			enum wb_reason stat);
>  void bdi_start_background_writeback(struct backing_dev_info *bdi);
>  int bdi_writeback_thread(void *data);
>  int bdi_has_dirty_io(struct backing_dev_info *bdi);
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index f1bfa12e..bdda069 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -50,6 +50,24 @@ enum writeback_sync_modes {
>  };
>  
>  /*
> + * why this writeback was initiated
> + */
> +enum wb_reason {
> +	/* The following are counts of pages written for a specific cause */
> +	WB_REASON_BALANCE_DIRTY,
> +	WB_REASON_BACKGROUND,
> +	WB_REASON_TRY_TO_FREE_PAGES,
> +	WB_REASON_SYNC,
> +	WB_REASON_PERIODIC,
> +	WB_REASON_LAPTOP_TIMER,
> +	WB_REASON_FREE_MORE_MEM,
> +	WB_REASON_FS_FREE_SPACE,
> +	WB_REASON_FORKER_THREAD,
> +
> +	WB_REASON_MAX,
> +};
> +
> +/*
>   * A control structure which tells the writeback code what to do.  These are
>   * always on the stack, and hence need no locking.  They are always initialised
>   * in a manner such that unspecified fields are set to zero.
> @@ -80,14 +98,17 @@ struct writeback_control {
>   */	
>  struct bdi_writeback;
>  int inode_wait(void *);
> -void writeback_inodes_sb(struct super_block *);
> -void writeback_inodes_sb_nr(struct super_block *, unsigned long nr);
> -int writeback_inodes_sb_if_idle(struct super_block *);
> -int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr);
> +void writeback_inodes_sb(struct super_block *, enum wb_reason stat);
> +void writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
> +							enum wb_reason stat);
> +int writeback_inodes_sb_if_idle(struct super_block *, enum wb_reason stat);
> +int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr,
> +							enum wb_reason stat);
>  void sync_inodes_sb(struct super_block *);
> -long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages);
> +long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
> +				enum wb_reason stat);
>  long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
> -void wakeup_flusher_threads(long nr_pages);
> +void wakeup_flusher_threads(long nr_pages, enum wb_reason stat);
>  
>  /* writeback.h requires fs.h; it, too, is not included from here. */
>  static inline void wait_on_inode(struct inode *inode)
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index 4565274..b8583cb 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -21,6 +21,19 @@
>  		{I_REFERENCED,		"I_REFERENCED"}		\
>  	)
>  
> +#define show_work_reason(reason)					\
> +	__print_symbolic(reason,					\
> +		{WB_REASON_BALANCE_DIRTY,	"balance_dirty"},	\
> +		{WB_REASON_BACKGROUND,		"background"},		\
> +		{WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"},	\
> +		{WB_REASON_SYNC,		"sync"},		\
> +		{WB_REASON_PERIODIC,		"periodic"},		\
> +		{WB_REASON_LAPTOP_TIMER,	"laptop_timer"},	\
> +		{WB_REASON_FREE_MORE_MEM,	"free_more_memory"},	\
> +		{WB_REASON_FS_FREE_SPACE,	"FS_free_space"},	\
> +		{WB_REASON_FORKER_THREAD,	"forker_thread"}	\
> +	)
> +
>  struct wb_writeback_work;
>  
>  DECLARE_EVENT_CLASS(writeback_work_class,
> @@ -34,6 +47,7 @@ DECLARE_EVENT_CLASS(writeback_work_class,
>  		__field(int, for_kupdate)
>  		__field(int, range_cyclic)
>  		__field(int, for_background)
> +		__field(int, reason)
>  	),
>  	TP_fast_assign(
>  		strncpy(__entry->name, dev_name(bdi->dev), 32);
> @@ -43,16 +57,18 @@ DECLARE_EVENT_CLASS(writeback_work_class,
>  		__entry->for_kupdate = work->for_kupdate;
>  		__entry->range_cyclic = work->range_cyclic;
>  		__entry->for_background	= work->for_background;
> +		__entry->reason = work->reason;
>  	),
>  	TP_printk("bdi %s: sb_dev %d:%d nr_pages=%ld sync_mode=%d "
> -		  "kupdate=%d range_cyclic=%d background=%d",
> +		  "kupdate=%d range_cyclic=%d background=%d reason=%s",
>  		  __entry->name,
>  		  MAJOR(__entry->sb_dev), MINOR(__entry->sb_dev),
>  		  __entry->nr_pages,
>  		  __entry->sync_mode,
>  		  __entry->for_kupdate,
>  		  __entry->range_cyclic,
> -		  __entry->for_background
> +		  __entry->for_background,
> +		  show_work_reason(__entry->reason)
>  	)
>  );
>  #define DEFINE_WRITEBACK_WORK_EVENT(name) \
> @@ -189,6 +205,7 @@ TRACE_EVENT(writeback_queue_io,
>  		__field(unsigned long,	older)
>  		__field(long,		age)
>  		__field(int,		moved)
> +		__field(int,		reason)
>  	),
>  	TP_fast_assign(
>  		unsigned long *older_than_this = work->older_than_this;
> @@ -197,12 +214,14 @@ TRACE_EVENT(writeback_queue_io,
>  		__entry->age	= older_than_this ?
>  				  (jiffies - *older_than_this) * 1000 / HZ : -1;
>  		__entry->moved	= moved;
> +		__entry->reason	= work->reason;
>  	),
> -	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d",
> +	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d reason=%s",
>  		__entry->name,
>  		__entry->older,	/* older_than_this in jiffies */
>  		__entry->age,	/* older_than_this in relative milliseconds */
> -		__entry->moved)
> +		__entry->moved,
> +		show_work_reason(__entry->reason))
>  );
>  
>  TRACE_EVENT(global_dirty_state,
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index d6edf8d..474bcfe 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -456,7 +456,8 @@ static int bdi_forker_thread(void *ptr)
>  				 * the bdi from the thread. Hopefully 1024 is
>  				 * large enough for efficient IO.
>  				 */
> -				writeback_inodes_wb(&bdi->wb, 1024);
> +				writeback_inodes_wb(&bdi->wb, 1024,
> +						    WB_REASON_FORKER_THREAD);
>  			} else {
>  				/*
>  				 * The spinlock makes sure we do not lose
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index d196074..0e78252 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -738,7 +738,8 @@ static void balance_dirty_pages(struct address_space *mapping,
>  		trace_balance_dirty_start(bdi);
>  		if (bdi_nr_reclaimable > task_bdi_thresh) {
>  			pages_written += writeback_inodes_wb(&bdi->wb,
> -							     write_chunk);
> +						write_chunk,
> +						WB_REASON_BALANCE_DIRTY);
>  			trace_balance_dirty_written(bdi, pages_written);
>  			if (pages_written >= write_chunk)
>  				break;		/* We've done our duty */
> @@ -909,7 +910,8 @@ void laptop_mode_timer_fn(unsigned long data)
>  	 * threshold
>  	 */
>  	if (bdi_has_dirty_io(&q->backing_dev_info))
> -		bdi_start_writeback(&q->backing_dev_info, nr_pages);
> +		bdi_start_writeback(&q->backing_dev_info, nr_pages,
> +					WB_REASON_LAPTOP_TIMER);
>  }
>  
>  /*
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 7ef6912..3b6c7ac 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -495,7 +495,6 @@ static pageout_t pageout(struct page *page, struct address_space *mapping,
>  			ClearPageReclaim(page);
>  			return PAGE_ACTIVATE;
>  		}
> -
>  		/*
>  		 * Wait on writeback if requested to. This happens when
>  		 * direct reclaiming a large contiguous area and the
> @@ -2198,7 +2197,8 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  		 */
>  		writeback_threshold = sc->nr_to_reclaim + sc->nr_to_reclaim / 2;
>  		if (total_scanned > writeback_threshold) {
> -			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned);
> +			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned,
> +						WB_REASON_TRY_TO_FREE_PAGES);
>  			sc->may_writepage = 1;
>  		}
>  
> -- 
> 1.7.3.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work
@ 2011-08-29 16:23     ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-29 16:23 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Mon 22-08-11 11:38:46, Curt Wohlgemuth wrote:
> This creates a new 'reason' field in a wb_writeback_work
> structure, which unambiguously identifies who initiates
> writeback activity.  A 'wb_reason' enumeration has been
> added to writeback.h, to enumerate the possible reasons.
> 
> The 'writeback_work_class' and tracepoint event class and
> 'writeback_queue_io' tracepoints are updated to include the
> symbolic 'reason' in all trace events.
> 
> And the 'writeback_inodes_sbXXX' family of routines has had
> a wb_stats parameter added to them, so callers can specify
> why writeback is being started.
  Looks good. You can add: Acked-by: Jan Kara <jack@suse.cz>

							Honza
> 
> Signed-off-by: Curt Wohlgemuth <curtw@google.com>
> ---
> 
> Changes since v2:
> 
>    - enum wb_stats renamed enum wb_reason
>    - All WB_STAT_xxx enum constants now named WB_REASON_xxx
>    - All 'reason' strings in tracepoints match the WB_REASON name
>    - The change to send 'work' to queue_io, move_expired_inodes, and
>      trace_writeback_queue_io are in a separate commit
> 
> 
>  fs/btrfs/extent-tree.c           |    3 ++-
>  fs/buffer.c                      |    2 +-
>  fs/ext4/inode.c                  |    2 +-
>  fs/fs-writeback.c                |   38 +++++++++++++++++++++++++-------------
>  fs/quota/quota.c                 |    2 +-
>  fs/sync.c                        |    4 ++--
>  fs/ubifs/budget.c                |    2 +-
>  include/linux/backing-dev.h      |    3 ++-
>  include/linux/writeback.h        |   33 +++++++++++++++++++++++++++------
>  include/trace/events/writeback.h |   27 +++++++++++++++++++++++----
>  mm/backing-dev.c                 |    3 ++-
>  mm/page-writeback.c              |    6 ++++--
>  mm/vmscan.c                      |    4 ++--
>  13 files changed, 93 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index f5be06a..c9ee0e1 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3340,7 +3340,8 @@ static int shrink_delalloc(struct btrfs_trans_handle *trans,
>  		smp_mb();
>  		nr_pages = min_t(unsigned long, nr_pages,
>  		       root->fs_info->delalloc_bytes >> PAGE_CACHE_SHIFT);
> -		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages);
> +		writeback_inodes_sb_nr_if_idle(root->fs_info->sb, nr_pages,
> +						WB_REASON_FS_FREE_SPACE);
>  
>  		spin_lock(&space_info->lock);
>  		if (reserved > space_info->bytes_reserved)
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 1a80b04..f5dcee6 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -285,7 +285,7 @@ static void free_more_memory(void)
>  	struct zone *zone;
>  	int nid;
>  
> -	wakeup_flusher_threads(1024);
> +	wakeup_flusher_threads(1024, WB_REASON_FREE_MORE_MEM);
>  	yield();
>  
>  	for_each_online_node(nid) {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c4da98a..f4f0f40 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2244,7 +2244,7 @@ static int ext4_nonda_switch(struct super_block *sb)
>  	 * start pushing delalloc when 1/2 of free blocks are dirty.
>  	 */
>  	if (free_blocks < 2 * dirty_blocks)
> -		writeback_inodes_sb_if_idle(sb);
> +		writeback_inodes_sb_if_idle(sb, WB_REASON_FS_FREE_SPACE);
>  
>  	return 0;
>  }
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index f1f5f65..a004fcd 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -41,6 +41,7 @@ struct wb_writeback_work {
>  	unsigned int for_kupdate:1;
>  	unsigned int range_cyclic:1;
>  	unsigned int for_background:1;
> +	enum wb_reason reason;		/* why was writeback initiated? */
>  
>  	struct list_head list;		/* pending work list */
>  	struct completion *done;	/* set if the caller waits */
> @@ -115,7 +116,7 @@ static void bdi_queue_work(struct backing_dev_info *bdi,
>  
>  static void
>  __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
> -		      bool range_cyclic)
> +		      bool range_cyclic, enum wb_reason reason)
>  {
>  	struct wb_writeback_work *work;
>  
> @@ -135,6 +136,7 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
>  	work->sync_mode	= WB_SYNC_NONE;
>  	work->nr_pages	= nr_pages;
>  	work->range_cyclic = range_cyclic;
> +	work->reason = reason;
>  
>  	bdi_queue_work(bdi, work);
>  }
> @@ -150,9 +152,10 @@ __bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
>   *   completion. Caller need not hold sb s_umount semaphore.
>   *
>   */
> -void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages)
> +void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
> +			enum wb_reason reason)
>  {
> -	__bdi_start_writeback(bdi, nr_pages, true);
> +	__bdi_start_writeback(bdi, nr_pages, true, reason);
>  }
>  
>  /**
> @@ -641,12 +644,14 @@ static long __writeback_inodes_wb(struct bdi_writeback *wb,
>  	return wrote;
>  }
>  
> -long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages)
> +long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
> +				enum wb_reason reason)
>  {
>  	struct wb_writeback_work work = {
>  		.nr_pages	= nr_pages,
>  		.sync_mode	= WB_SYNC_NONE,
>  		.range_cyclic	= 1,
> +		.reason		= reason,
>  	};
>  
>  	spin_lock(&wb->list_lock);
> @@ -818,6 +823,7 @@ static long wb_check_background_flush(struct bdi_writeback *wb)
>  			.sync_mode	= WB_SYNC_NONE,
>  			.for_background	= 1,
>  			.range_cyclic	= 1,
> +			.reason		= WB_REASON_BACKGROUND,
>  		};
>  
>  		return wb_writeback(wb, &work);
> @@ -851,6 +857,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
>  			.sync_mode	= WB_SYNC_NONE,
>  			.for_kupdate	= 1,
>  			.range_cyclic	= 1,
> +			.reason		= WB_REASON_PERIODIC,
>  		};
>  
>  		return wb_writeback(wb, &work);
> @@ -969,7 +976,7 @@ int bdi_writeback_thread(void *data)
>   * Start writeback of `nr_pages' pages.  If `nr_pages' is zero, write back
>   * the whole world.
>   */
> -void wakeup_flusher_threads(long nr_pages)
> +void wakeup_flusher_threads(long nr_pages, enum wb_reason reason)
>  {
>  	struct backing_dev_info *bdi;
>  
> @@ -982,7 +989,7 @@ void wakeup_flusher_threads(long nr_pages)
>  	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) {
>  		if (!bdi_has_dirty_io(bdi))
>  			continue;
> -		__bdi_start_writeback(bdi, nr_pages, false);
> +		__bdi_start_writeback(bdi, nr_pages, false, reason);
>  	}
>  	rcu_read_unlock();
>  }
> @@ -1203,7 +1210,9 @@ static void wait_sb_inodes(struct super_block *sb)
>   * on how many (if any) will be written, and this function does not wait
>   * for IO completion of submitted IO.
>   */
> -void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
> +void writeback_inodes_sb_nr(struct super_block *sb,
> +			    unsigned long nr,
> +			    enum wb_reason reason)
>  {
>  	DECLARE_COMPLETION_ONSTACK(done);
>  	struct wb_writeback_work work = {
> @@ -1212,6 +1221,7 @@ void writeback_inodes_sb_nr(struct super_block *sb, unsigned long nr)
>  		.tagged_writepages	= 1,
>  		.done			= &done,
>  		.nr_pages		= nr,
> +		.reason			= reason,
>  	};
>  
>  	WARN_ON(!rwsem_is_locked(&sb->s_umount));
> @@ -1228,9 +1238,9 @@ EXPORT_SYMBOL(writeback_inodes_sb_nr);
>   * on how many (if any) will be written, and this function does not wait
>   * for IO completion of submitted IO.
>   */
> -void writeback_inodes_sb(struct super_block *sb)
> +void writeback_inodes_sb(struct super_block *sb, enum wb_reason reason)
>  {
> -	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages());
> +	return writeback_inodes_sb_nr(sb, get_nr_dirty_pages(), reason);
>  }
>  EXPORT_SYMBOL(writeback_inodes_sb);
>  
> @@ -1241,11 +1251,11 @@ EXPORT_SYMBOL(writeback_inodes_sb);
>   * Invoke writeback_inodes_sb if no writeback is currently underway.
>   * Returns 1 if writeback was started, 0 if not.
>   */
> -int writeback_inodes_sb_if_idle(struct super_block *sb)
> +int writeback_inodes_sb_if_idle(struct super_block *sb, enum wb_reason reason)
>  {
>  	if (!writeback_in_progress(sb->s_bdi)) {
>  		down_read(&sb->s_umount);
> -		writeback_inodes_sb(sb);
> +		writeback_inodes_sb(sb, reason);
>  		up_read(&sb->s_umount);
>  		return 1;
>  	} else
> @@ -1262,11 +1272,12 @@ EXPORT_SYMBOL(writeback_inodes_sb_if_idle);
>   * Returns 1 if writeback was started, 0 if not.
>   */
>  int writeback_inodes_sb_nr_if_idle(struct super_block *sb,
> -				   unsigned long nr)
> +				   unsigned long nr,
> +				   enum wb_reason reason)
>  {
>  	if (!writeback_in_progress(sb->s_bdi)) {
>  		down_read(&sb->s_umount);
> -		writeback_inodes_sb_nr(sb, nr);
> +		writeback_inodes_sb_nr(sb, nr, reason);
>  		up_read(&sb->s_umount);
>  		return 1;
>  	} else
> @@ -1290,6 +1301,7 @@ void sync_inodes_sb(struct super_block *sb)
>  		.nr_pages	= LONG_MAX,
>  		.range_cyclic	= 0,
>  		.done		= &done,
> +		.reason		= WB_REASON_SYNC,
>  	};
>  
>  	WARN_ON(!rwsem_is_locked(&sb->s_umount));
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index b34bdb2..b7a4366 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -286,7 +286,7 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
>  		/* caller already holds s_umount */
>  		if (sb->s_flags & MS_RDONLY)
>  			return -EROFS;
> -		writeback_inodes_sb(sb);
> +		writeback_inodes_sb(sb, WB_REASON_SYNC);
>  		return 0;
>  	default:
>  		return -EINVAL;
> diff --git a/fs/sync.c b/fs/sync.c
> index c98a747..101b8ef 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -43,7 +43,7 @@ static int __sync_filesystem(struct super_block *sb, int wait)
>  	if (wait)
>  		sync_inodes_sb(sb);
>  	else
> -		writeback_inodes_sb(sb);
> +		writeback_inodes_sb(sb, WB_REASON_SYNC);
>  
>  	if (sb->s_op->sync_fs)
>  		sb->s_op->sync_fs(sb, wait);
> @@ -98,7 +98,7 @@ static void sync_filesystems(int wait)
>   */
>  SYSCALL_DEFINE0(sync)
>  {
> -	wakeup_flusher_threads(0);
> +	wakeup_flusher_threads(0, WB_REASON_SYNC);
>  	sync_filesystems(0);
>  	sync_filesystems(1);
>  	if (unlikely(laptop_mode))
> diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
> index 315de66..bc4f94b 100644
> --- a/fs/ubifs/budget.c
> +++ b/fs/ubifs/budget.c
> @@ -63,7 +63,7 @@
>  static void shrink_liability(struct ubifs_info *c, int nr_to_write)
>  {
>  	down_read(&c->vfs_sb->s_umount);
> -	writeback_inodes_sb(c->vfs_sb);
> +	writeback_inodes_sb(c->vfs_sb, WB_REASON_FS_FREE_SPACE);
>  	up_read(&c->vfs_sb->s_umount);
>  }
>  
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index 3b2f9cb..ef85559 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -107,7 +107,8 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent,
>  int bdi_register_dev(struct backing_dev_info *bdi, dev_t dev);
>  void bdi_unregister(struct backing_dev_info *bdi);
>  int bdi_setup_and_register(struct backing_dev_info *, char *, unsigned int);
> -void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages);
> +void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
> +			enum wb_reason stat);
>  void bdi_start_background_writeback(struct backing_dev_info *bdi);
>  int bdi_writeback_thread(void *data);
>  int bdi_has_dirty_io(struct backing_dev_info *bdi);
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index f1bfa12e..bdda069 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -50,6 +50,24 @@ enum writeback_sync_modes {
>  };
>  
>  /*
> + * why this writeback was initiated
> + */
> +enum wb_reason {
> +	/* The following are counts of pages written for a specific cause */
> +	WB_REASON_BALANCE_DIRTY,
> +	WB_REASON_BACKGROUND,
> +	WB_REASON_TRY_TO_FREE_PAGES,
> +	WB_REASON_SYNC,
> +	WB_REASON_PERIODIC,
> +	WB_REASON_LAPTOP_TIMER,
> +	WB_REASON_FREE_MORE_MEM,
> +	WB_REASON_FS_FREE_SPACE,
> +	WB_REASON_FORKER_THREAD,
> +
> +	WB_REASON_MAX,
> +};
> +
> +/*
>   * A control structure which tells the writeback code what to do.  These are
>   * always on the stack, and hence need no locking.  They are always initialised
>   * in a manner such that unspecified fields are set to zero.
> @@ -80,14 +98,17 @@ struct writeback_control {
>   */	
>  struct bdi_writeback;
>  int inode_wait(void *);
> -void writeback_inodes_sb(struct super_block *);
> -void writeback_inodes_sb_nr(struct super_block *, unsigned long nr);
> -int writeback_inodes_sb_if_idle(struct super_block *);
> -int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr);
> +void writeback_inodes_sb(struct super_block *, enum wb_reason stat);
> +void writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
> +							enum wb_reason stat);
> +int writeback_inodes_sb_if_idle(struct super_block *, enum wb_reason stat);
> +int writeback_inodes_sb_nr_if_idle(struct super_block *, unsigned long nr,
> +							enum wb_reason stat);
>  void sync_inodes_sb(struct super_block *);
> -long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages);
> +long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
> +				enum wb_reason stat);
>  long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
> -void wakeup_flusher_threads(long nr_pages);
> +void wakeup_flusher_threads(long nr_pages, enum wb_reason stat);
>  
>  /* writeback.h requires fs.h; it, too, is not included from here. */
>  static inline void wait_on_inode(struct inode *inode)
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index 4565274..b8583cb 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -21,6 +21,19 @@
>  		{I_REFERENCED,		"I_REFERENCED"}		\
>  	)
>  
> +#define show_work_reason(reason)					\
> +	__print_symbolic(reason,					\
> +		{WB_REASON_BALANCE_DIRTY,	"balance_dirty"},	\
> +		{WB_REASON_BACKGROUND,		"background"},		\
> +		{WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"},	\
> +		{WB_REASON_SYNC,		"sync"},		\
> +		{WB_REASON_PERIODIC,		"periodic"},		\
> +		{WB_REASON_LAPTOP_TIMER,	"laptop_timer"},	\
> +		{WB_REASON_FREE_MORE_MEM,	"free_more_memory"},	\
> +		{WB_REASON_FS_FREE_SPACE,	"FS_free_space"},	\
> +		{WB_REASON_FORKER_THREAD,	"forker_thread"}	\
> +	)
> +
>  struct wb_writeback_work;
>  
>  DECLARE_EVENT_CLASS(writeback_work_class,
> @@ -34,6 +47,7 @@ DECLARE_EVENT_CLASS(writeback_work_class,
>  		__field(int, for_kupdate)
>  		__field(int, range_cyclic)
>  		__field(int, for_background)
> +		__field(int, reason)
>  	),
>  	TP_fast_assign(
>  		strncpy(__entry->name, dev_name(bdi->dev), 32);
> @@ -43,16 +57,18 @@ DECLARE_EVENT_CLASS(writeback_work_class,
>  		__entry->for_kupdate = work->for_kupdate;
>  		__entry->range_cyclic = work->range_cyclic;
>  		__entry->for_background	= work->for_background;
> +		__entry->reason = work->reason;
>  	),
>  	TP_printk("bdi %s: sb_dev %d:%d nr_pages=%ld sync_mode=%d "
> -		  "kupdate=%d range_cyclic=%d background=%d",
> +		  "kupdate=%d range_cyclic=%d background=%d reason=%s",
>  		  __entry->name,
>  		  MAJOR(__entry->sb_dev), MINOR(__entry->sb_dev),
>  		  __entry->nr_pages,
>  		  __entry->sync_mode,
>  		  __entry->for_kupdate,
>  		  __entry->range_cyclic,
> -		  __entry->for_background
> +		  __entry->for_background,
> +		  show_work_reason(__entry->reason)
>  	)
>  );
>  #define DEFINE_WRITEBACK_WORK_EVENT(name) \
> @@ -189,6 +205,7 @@ TRACE_EVENT(writeback_queue_io,
>  		__field(unsigned long,	older)
>  		__field(long,		age)
>  		__field(int,		moved)
> +		__field(int,		reason)
>  	),
>  	TP_fast_assign(
>  		unsigned long *older_than_this = work->older_than_this;
> @@ -197,12 +214,14 @@ TRACE_EVENT(writeback_queue_io,
>  		__entry->age	= older_than_this ?
>  				  (jiffies - *older_than_this) * 1000 / HZ : -1;
>  		__entry->moved	= moved;
> +		__entry->reason	= work->reason;
>  	),
> -	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d",
> +	TP_printk("bdi %s: older=%lu age=%ld enqueue=%d reason=%s",
>  		__entry->name,
>  		__entry->older,	/* older_than_this in jiffies */
>  		__entry->age,	/* older_than_this in relative milliseconds */
> -		__entry->moved)
> +		__entry->moved,
> +		show_work_reason(__entry->reason))
>  );
>  
>  TRACE_EVENT(global_dirty_state,
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index d6edf8d..474bcfe 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -456,7 +456,8 @@ static int bdi_forker_thread(void *ptr)
>  				 * the bdi from the thread. Hopefully 1024 is
>  				 * large enough for efficient IO.
>  				 */
> -				writeback_inodes_wb(&bdi->wb, 1024);
> +				writeback_inodes_wb(&bdi->wb, 1024,
> +						    WB_REASON_FORKER_THREAD);
>  			} else {
>  				/*
>  				 * The spinlock makes sure we do not lose
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index d196074..0e78252 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -738,7 +738,8 @@ static void balance_dirty_pages(struct address_space *mapping,
>  		trace_balance_dirty_start(bdi);
>  		if (bdi_nr_reclaimable > task_bdi_thresh) {
>  			pages_written += writeback_inodes_wb(&bdi->wb,
> -							     write_chunk);
> +						write_chunk,
> +						WB_REASON_BALANCE_DIRTY);
>  			trace_balance_dirty_written(bdi, pages_written);
>  			if (pages_written >= write_chunk)
>  				break;		/* We've done our duty */
> @@ -909,7 +910,8 @@ void laptop_mode_timer_fn(unsigned long data)
>  	 * threshold
>  	 */
>  	if (bdi_has_dirty_io(&q->backing_dev_info))
> -		bdi_start_writeback(&q->backing_dev_info, nr_pages);
> +		bdi_start_writeback(&q->backing_dev_info, nr_pages,
> +					WB_REASON_LAPTOP_TIMER);
>  }
>  
>  /*
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 7ef6912..3b6c7ac 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -495,7 +495,6 @@ static pageout_t pageout(struct page *page, struct address_space *mapping,
>  			ClearPageReclaim(page);
>  			return PAGE_ACTIVATE;
>  		}
> -
>  		/*
>  		 * Wait on writeback if requested to. This happens when
>  		 * direct reclaiming a large contiguous area and the
> @@ -2198,7 +2197,8 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  		 */
>  		writeback_threshold = sc->nr_to_reclaim + sc->nr_to_reclaim / 2;
>  		if (total_scanned > writeback_threshold) {
> -			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned);
> +			wakeup_flusher_threads(laptop_mode ? 0 : total_scanned,
> +						WB_REASON_TRY_TO_FREE_PAGES);
>  			sc->may_writepage = 1;
>  		}
>  
> -- 
> 1.7.3.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work
  2011-08-29 16:23     ` Jan Kara
  (?)
@ 2011-08-29 16:34     ` Jan Kara
  2011-08-30 18:06         ` Curt Wohlgemuth
  -1 siblings, 1 reply; 18+ messages in thread
From: Jan Kara @ 2011-08-29 16:34 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Mon 29-08-11 18:23:13, Jan Kara wrote:
> On Mon 22-08-11 11:38:46, Curt Wohlgemuth wrote:
> > This creates a new 'reason' field in a wb_writeback_work
> > structure, which unambiguously identifies who initiates
> > writeback activity.  A 'wb_reason' enumeration has been
> > added to writeback.h, to enumerate the possible reasons.
> > 
> > The 'writeback_work_class' and tracepoint event class and
> > 'writeback_queue_io' tracepoints are updated to include the
> > symbolic 'reason' in all trace events.
> > 
> > And the 'writeback_inodes_sbXXX' family of routines has had
> > a wb_stats parameter added to them, so callers can specify
> > why writeback is being started.
>   Looks good. You can add: Acked-by: Jan Kara <jack@suse.cz>
  Oh, one small typo correction:

> > +#define show_work_reason(reason)					\
> > +	__print_symbolic(reason,					\
> > +		{WB_REASON_BALANCE_DIRTY,	"balance_dirty"},	\
> > +		{WB_REASON_BACKGROUND,		"background"},		\
> > +		{WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"},	\
> > +		{WB_REASON_SYNC,		"sync"},		\
> > +		{WB_REASON_PERIODIC,		"periodic"},		\
> > +		{WB_REASON_LAPTOP_TIMER,	"laptop_timer"},	\
> > +		{WB_REASON_FREE_MORE_MEM,	"free_more_memory"},	\
> > +		{WB_REASON_FS_FREE_SPACE,	"FS_free_space"},	\
                                                 ^^ should be in
non-capital letters?

> > +		{WB_REASON_FORKER_THREAD,	"forker_thread"}	\
> > +	)

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/3 v3] writeback: Add writeback stats for pages written
  2011-08-22 18:38   ` Curt Wohlgemuth
@ 2011-08-29 16:36     ` Jan Kara
  -1 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-29 16:36 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Mon 22-08-11 11:38:47, Curt Wohlgemuth wrote:
> Add a new file, /proc/writeback, which displays
> machine global data for how many pages were cleaned for
> which reasons.
  I'm not sure about the placement in /proc/writeback - maybe I'd be
happier if it was somewhere under /sys/kernel/debug but I don't really have
a better suggestion and I don't care that much either. Maybe Christoph or
Andrew have some idea?

...
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index bdda069..5168ac9 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -59,6 +59,7 @@ enum wb_reason {
>  	WB_REASON_TRY_TO_FREE_PAGES,
>  	WB_REASON_SYNC,
>  	WB_REASON_PERIODIC,
> +	WB_REASON_FDATAWRITE,
>  	WB_REASON_LAPTOP_TIMER,
>  	WB_REASON_FREE_MORE_MEM,
>  	WB_REASON_FS_FREE_SPACE,
> @@ -67,6 +68,7 @@ enum wb_reason {
>  	WB_REASON_MAX,
>  };
>  
> +
  The additional empty line doesn't make much sense here?

>  /*
>   * A control structure which tells the writeback code what to do.  These are
>   * always on the stack, and hence need no locking.  They are always initialised
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index 474bcfe..6613391 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
...
> @@ -56,9 +60,77 @@ void bdi_lock_two(struct bdi_writeback *wb1, struct bdi_writeback *wb2)
>  	}
>  }
>  
> +
  And another empty line here?

> +static const char *wb_stats_labels[WB_REASON_MAX] = {
> +	[WB_REASON_BALANCE_DIRTY] = "page: balance_dirty_pages",
> +	[WB_REASON_BACKGROUND] = "page: background_writeout",
> +	[WB_REASON_TRY_TO_FREE_PAGES] = "page: try_to_free_pages",
> +	[WB_REASON_SYNC] = "page: sync",
> +	[WB_REASON_PERIODIC] = "page: periodic",
> +	[WB_REASON_FDATAWRITE] = "page: fdatawrite",
> +	[WB_REASON_LAPTOP_TIMER] = "page: laptop_periodic",
> +	[WB_REASON_FREE_MORE_MEM] = "page: free_more_memory",
> +	[WB_REASON_FS_FREE_SPACE] = "page: fs_free_space",
> +};
 I don't think it's good to have two enum->string translation tables for
reasons. That's prone to errors which is in fact proven by the fact that
you ommitted FORKER_THREAD reason here.
  
> @@ -157,6 +248,7 @@ static inline void bdi_debug_unregister(struct backing_dev_info *bdi)
>  }
>  #endif
>  
> +
  Another empty line here? You seem to like them ;)

>  static ssize_t read_ahead_kb_store(struct device *dev,
>  				  struct device_attribute *attr,
>  				  const char *buf, size_t count)

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/3 v3] writeback: Add writeback stats for pages written
@ 2011-08-29 16:36     ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-29 16:36 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Christoph Hellwig, Wu Fengguang, Jan Kara, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Mon 22-08-11 11:38:47, Curt Wohlgemuth wrote:
> Add a new file, /proc/writeback, which displays
> machine global data for how many pages were cleaned for
> which reasons.
  I'm not sure about the placement in /proc/writeback - maybe I'd be
happier if it was somewhere under /sys/kernel/debug but I don't really have
a better suggestion and I don't care that much either. Maybe Christoph or
Andrew have some idea?

...
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index bdda069..5168ac9 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -59,6 +59,7 @@ enum wb_reason {
>  	WB_REASON_TRY_TO_FREE_PAGES,
>  	WB_REASON_SYNC,
>  	WB_REASON_PERIODIC,
> +	WB_REASON_FDATAWRITE,
>  	WB_REASON_LAPTOP_TIMER,
>  	WB_REASON_FREE_MORE_MEM,
>  	WB_REASON_FS_FREE_SPACE,
> @@ -67,6 +68,7 @@ enum wb_reason {
>  	WB_REASON_MAX,
>  };
>  
> +
  The additional empty line doesn't make much sense here?

>  /*
>   * A control structure which tells the writeback code what to do.  These are
>   * always on the stack, and hence need no locking.  They are always initialised
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index 474bcfe..6613391 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
...
> @@ -56,9 +60,77 @@ void bdi_lock_two(struct bdi_writeback *wb1, struct bdi_writeback *wb2)
>  	}
>  }
>  
> +
  And another empty line here?

> +static const char *wb_stats_labels[WB_REASON_MAX] = {
> +	[WB_REASON_BALANCE_DIRTY] = "page: balance_dirty_pages",
> +	[WB_REASON_BACKGROUND] = "page: background_writeout",
> +	[WB_REASON_TRY_TO_FREE_PAGES] = "page: try_to_free_pages",
> +	[WB_REASON_SYNC] = "page: sync",
> +	[WB_REASON_PERIODIC] = "page: periodic",
> +	[WB_REASON_FDATAWRITE] = "page: fdatawrite",
> +	[WB_REASON_LAPTOP_TIMER] = "page: laptop_periodic",
> +	[WB_REASON_FREE_MORE_MEM] = "page: free_more_memory",
> +	[WB_REASON_FS_FREE_SPACE] = "page: fs_free_space",
> +};
 I don't think it's good to have two enum->string translation tables for
reasons. That's prone to errors which is in fact proven by the fact that
you ommitted FORKER_THREAD reason here.
  
> @@ -157,6 +248,7 @@ static inline void bdi_debug_unregister(struct backing_dev_info *bdi)
>  }
>  #endif
>  
> +
  Another empty line here? You seem to like them ;)

>  static ssize_t read_ahead_kb_store(struct device *dev,
>  				  struct device_attribute *attr,
>  				  const char *buf, size_t count)

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work
  2011-08-29 16:34     ` Jan Kara
@ 2011-08-30 18:06         ` Curt Wohlgemuth
  0 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-30 18:06 UTC (permalink / raw)
  To: Jan Kara
  Cc: Christoph Hellwig, Wu Fengguang, Andrew Morton, Dave Chinner,
	Michael Rubin, linux-fsdevel, linux-mm

On Mon, Aug 29, 2011 at 9:34 AM, Jan Kara <jack@suse.cz> wrote:
> On Mon 29-08-11 18:23:13, Jan Kara wrote:
>> On Mon 22-08-11 11:38:46, Curt Wohlgemuth wrote:
>> > This creates a new 'reason' field in a wb_writeback_work
>> > structure, which unambiguously identifies who initiates
>> > writeback activity.  A 'wb_reason' enumeration has been
>> > added to writeback.h, to enumerate the possible reasons.
>> >
>> > The 'writeback_work_class' and tracepoint event class and
>> > 'writeback_queue_io' tracepoints are updated to include the
>> > symbolic 'reason' in all trace events.
>> >
>> > And the 'writeback_inodes_sbXXX' family of routines has had
>> > a wb_stats parameter added to them, so callers can specify
>> > why writeback is being started.
>>   Looks good. You can add: Acked-by: Jan Kara <jack@suse.cz>
>  Oh, one small typo correction:
>
>> > +#define show_work_reason(reason)                                   \
>> > +   __print_symbolic(reason,                                        \
>> > +           {WB_REASON_BALANCE_DIRTY,       "balance_dirty"},       \
>> > +           {WB_REASON_BACKGROUND,          "background"},          \
>> > +           {WB_REASON_TRY_TO_FREE_PAGES,   "try_to_free_pages"},   \
>> > +           {WB_REASON_SYNC,                "sync"},                \
>> > +           {WB_REASON_PERIODIC,            "periodic"},            \
>> > +           {WB_REASON_LAPTOP_TIMER,        "laptop_timer"},        \
>> > +           {WB_REASON_FREE_MORE_MEM,       "free_more_memory"},    \
>> > +           {WB_REASON_FS_FREE_SPACE,       "FS_free_space"},       \
>                                                 ^^ should be in
> non-capital letters?

Oops, right, thanks for catching this.

Curt

>> > +           {WB_REASON_FORKER_THREAD,       "forker_thread"}        \
>> > +   )
>
>                                                                Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work
@ 2011-08-30 18:06         ` Curt Wohlgemuth
  0 siblings, 0 replies; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-30 18:06 UTC (permalink / raw)
  To: Jan Kara
  Cc: Christoph Hellwig, Wu Fengguang, Andrew Morton, Dave Chinner,
	Michael Rubin, linux-fsdevel, linux-mm

On Mon, Aug 29, 2011 at 9:34 AM, Jan Kara <jack@suse.cz> wrote:
> On Mon 29-08-11 18:23:13, Jan Kara wrote:
>> On Mon 22-08-11 11:38:46, Curt Wohlgemuth wrote:
>> > This creates a new 'reason' field in a wb_writeback_work
>> > structure, which unambiguously identifies who initiates
>> > writeback activity.  A 'wb_reason' enumeration has been
>> > added to writeback.h, to enumerate the possible reasons.
>> >
>> > The 'writeback_work_class' and tracepoint event class and
>> > 'writeback_queue_io' tracepoints are updated to include the
>> > symbolic 'reason' in all trace events.
>> >
>> > And the 'writeback_inodes_sbXXX' family of routines has had
>> > a wb_stats parameter added to them, so callers can specify
>> > why writeback is being started.
>>   Looks good. You can add: Acked-by: Jan Kara <jack@suse.cz>
>  Oh, one small typo correction:
>
>> > +#define show_work_reason(reason)                                   \
>> > +   __print_symbolic(reason,                                        \
>> > +           {WB_REASON_BALANCE_DIRTY,       "balance_dirty"},       \
>> > +           {WB_REASON_BACKGROUND,          "background"},          \
>> > +           {WB_REASON_TRY_TO_FREE_PAGES,   "try_to_free_pages"},   \
>> > +           {WB_REASON_SYNC,                "sync"},                \
>> > +           {WB_REASON_PERIODIC,            "periodic"},            \
>> > +           {WB_REASON_LAPTOP_TIMER,        "laptop_timer"},        \
>> > +           {WB_REASON_FREE_MORE_MEM,       "free_more_memory"},    \
>> > +           {WB_REASON_FS_FREE_SPACE,       "FS_free_space"},       \
>                                                 ^^ should be in
> non-capital letters?

Oops, right, thanks for catching this.

Curt

>> > +           {WB_REASON_FORKER_THREAD,       "forker_thread"}        \
>> > +   )
>
>                                                                Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/3 v3] writeback: Add writeback stats for pages written
  2011-08-29 16:36     ` Jan Kara
  (?)
@ 2011-08-30 18:13     ` Curt Wohlgemuth
  2011-08-30 22:24         ` Jan Kara
  -1 siblings, 1 reply; 18+ messages in thread
From: Curt Wohlgemuth @ 2011-08-30 18:13 UTC (permalink / raw)
  To: Jan Kara
  Cc: Christoph Hellwig, Wu Fengguang, Andrew Morton, Dave Chinner,
	Michael Rubin, linux-fsdevel, linux-mm

Hi Jan:

On Mon, Aug 29, 2011 at 9:36 AM, Jan Kara <jack@suse.cz> wrote:
> On Mon 22-08-11 11:38:47, Curt Wohlgemuth wrote:
>> Add a new file, /proc/writeback, which displays
>> machine global data for how many pages were cleaned for
>> which reasons.
>  I'm not sure about the placement in /proc/writeback - maybe I'd be
> happier if it was somewhere under /sys/kernel/debug but I don't really have
> a better suggestion and I don't care that much either. Maybe Christoph or
> Andrew have some idea?

I'm open to suggestions...

> ...
>> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
>> index bdda069..5168ac9 100644
>> --- a/include/linux/writeback.h
>> +++ b/include/linux/writeback.h
>> @@ -59,6 +59,7 @@ enum wb_reason {
>>       WB_REASON_TRY_TO_FREE_PAGES,
>>       WB_REASON_SYNC,
>>       WB_REASON_PERIODIC,
>> +     WB_REASON_FDATAWRITE,
>>       WB_REASON_LAPTOP_TIMER,
>>       WB_REASON_FREE_MORE_MEM,
>>       WB_REASON_FS_FREE_SPACE,
>> @@ -67,6 +68,7 @@ enum wb_reason {
>>       WB_REASON_MAX,
>>  };
>>
>> +
>  The additional empty line doesn't make much sense here?

Sigh.  Yes, I do like whitespace, but not usually this much; I'll fix them all.

>>  /*
>>   * A control structure which tells the writeback code what to do.  These are
>>   * always on the stack, and hence need no locking.  They are always initialised
>> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
>> index 474bcfe..6613391 100644
>> --- a/mm/backing-dev.c
>> +++ b/mm/backing-dev.c
> ...
>> @@ -56,9 +60,77 @@ void bdi_lock_two(struct bdi_writeback *wb1, struct bdi_writeback *wb2)
>>       }
>>  }
>>
>> +
>  And another empty line here?
>
>> +static const char *wb_stats_labels[WB_REASON_MAX] = {
>> +     [WB_REASON_BALANCE_DIRTY] = "page: balance_dirty_pages",
>> +     [WB_REASON_BACKGROUND] = "page: background_writeout",
>> +     [WB_REASON_TRY_TO_FREE_PAGES] = "page: try_to_free_pages",
>> +     [WB_REASON_SYNC] = "page: sync",
>> +     [WB_REASON_PERIODIC] = "page: periodic",
>> +     [WB_REASON_FDATAWRITE] = "page: fdatawrite",
>> +     [WB_REASON_LAPTOP_TIMER] = "page: laptop_periodic",
>> +     [WB_REASON_FREE_MORE_MEM] = "page: free_more_memory",
>> +     [WB_REASON_FS_FREE_SPACE] = "page: fs_free_space",
>> +};
>  I don't think it's good to have two enum->string translation tables for
> reasons. That's prone to errors which is in fact proven by the fact that
> you ommitted FORKER_THREAD reason here.

Ah, thanks for catching the omitted reason.  I assume you mean the
table above, and

   +#define show_work_reason(reason)

from the patch 2/3 (in the trace events file).  Hmm, that could be a
challenge, given the limitations on what you can do in trace macros.
I'll think on this though.

Thanks,
Curt

>
>> @@ -157,6 +248,7 @@ static inline void bdi_debug_unregister(struct backing_dev_info *bdi)
>>  }
>>  #endif
>>
>> +
>  Another empty line here? You seem to like them ;)
>
>>  static ssize_t read_ahead_kb_store(struct device *dev,
>>                                 struct device_attribute *attr,
>>                                 const char *buf, size_t count)
>
>                                                                        Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/3 v3] writeback: Add writeback stats for pages written
  2011-08-30 18:13     ` Curt Wohlgemuth
@ 2011-08-30 22:24         ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-30 22:24 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Jan Kara, Christoph Hellwig, Wu Fengguang, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Tue 30-08-11 11:13:50, Curt Wohlgemuth wrote:
> Hi Jan:
> 
> >> +static const char *wb_stats_labels[WB_REASON_MAX] = {
> >> +     [WB_REASON_BALANCE_DIRTY] = "page: balance_dirty_pages",
> >> +     [WB_REASON_BACKGROUND] = "page: background_writeout",
> >> +     [WB_REASON_TRY_TO_FREE_PAGES] = "page: try_to_free_pages",
> >> +     [WB_REASON_SYNC] = "page: sync",
> >> +     [WB_REASON_PERIODIC] = "page: periodic",
> >> +     [WB_REASON_FDATAWRITE] = "page: fdatawrite",
> >> +     [WB_REASON_LAPTOP_TIMER] = "page: laptop_periodic",
> >> +     [WB_REASON_FREE_MORE_MEM] = "page: free_more_memory",
> >> +     [WB_REASON_FS_FREE_SPACE] = "page: fs_free_space",
> >> +};
> >  I don't think it's good to have two enum->string translation tables for
> > reasons. That's prone to errors which is in fact proven by the fact that
> > you ommitted FORKER_THREAD reason here.
> 
> Ah, thanks for catching the omitted reason.  I assume you mean the
> table above, and
> 
>    +#define show_work_reason(reason)
> 
> from the patch 2/3 (in the trace events file).  Hmm, that could be a
> challenge, given the limitations on what you can do in trace macros.
> I'll think on this though.
  Ah right, with trace points it's not so simple. I guess we could have
an array with labels as a global symbol and dereference it in tracepoints.

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/3 v3] writeback: Add writeback stats for pages written
@ 2011-08-30 22:24         ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2011-08-30 22:24 UTC (permalink / raw)
  To: Curt Wohlgemuth
  Cc: Jan Kara, Christoph Hellwig, Wu Fengguang, Andrew Morton,
	Dave Chinner, Michael Rubin, linux-fsdevel, linux-mm

On Tue 30-08-11 11:13:50, Curt Wohlgemuth wrote:
> Hi Jan:
> 
> >> +static const char *wb_stats_labels[WB_REASON_MAX] = {
> >> +     [WB_REASON_BALANCE_DIRTY] = "page: balance_dirty_pages",
> >> +     [WB_REASON_BACKGROUND] = "page: background_writeout",
> >> +     [WB_REASON_TRY_TO_FREE_PAGES] = "page: try_to_free_pages",
> >> +     [WB_REASON_SYNC] = "page: sync",
> >> +     [WB_REASON_PERIODIC] = "page: periodic",
> >> +     [WB_REASON_FDATAWRITE] = "page: fdatawrite",
> >> +     [WB_REASON_LAPTOP_TIMER] = "page: laptop_periodic",
> >> +     [WB_REASON_FREE_MORE_MEM] = "page: free_more_memory",
> >> +     [WB_REASON_FS_FREE_SPACE] = "page: fs_free_space",
> >> +};
> >  I don't think it's good to have two enum->string translation tables for
> > reasons. That's prone to errors which is in fact proven by the fact that
> > you ommitted FORKER_THREAD reason here.
> 
> Ah, thanks for catching the omitted reason.  I assume you mean the
> table above, and
> 
>    +#define show_work_reason(reason)
> 
> from the patch 2/3 (in the trace events file).  Hmm, that could be a
> challenge, given the limitations on what you can do in trace macros.
> I'll think on this though.
  Ah right, with trace points it's not so simple. I guess we could have
an array with labels as a global symbol and dereference it in tracepoints.

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2011-08-30 22:24 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-22 18:38 [PATCH 1/3] writeback: send work item to queue_io, move_expired_inodes Curt Wohlgemuth
2011-08-22 18:38 ` Curt Wohlgemuth
2011-08-22 18:38 ` [PATCH 2/3 v3] writeback: Add a 'reason' to wb_writeback_work Curt Wohlgemuth
2011-08-22 18:38   ` Curt Wohlgemuth
2011-08-29 16:23   ` Jan Kara
2011-08-29 16:23     ` Jan Kara
2011-08-29 16:34     ` Jan Kara
2011-08-30 18:06       ` Curt Wohlgemuth
2011-08-30 18:06         ` Curt Wohlgemuth
2011-08-22 18:38 ` [PATCH 3/3 v3] writeback: Add writeback stats for pages written Curt Wohlgemuth
2011-08-22 18:38   ` Curt Wohlgemuth
2011-08-29 16:36   ` Jan Kara
2011-08-29 16:36     ` Jan Kara
2011-08-30 18:13     ` Curt Wohlgemuth
2011-08-30 22:24       ` Jan Kara
2011-08-30 22:24         ` Jan Kara
2011-08-29 16:21 ` [PATCH 1/3] writeback: send work item to queue_io, move_expired_inodes Jan Kara
2011-08-29 16:21   ` Jan Kara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.