writeback visibility

Debugging writeback issues and tuning an application's writeback activity is easier when the activity is visible. With large clusters, classifying and root causing writeback problems has been a big headache. This patch series contains a series of patches that our team has been using to start getting a handle on writeback behaviour. These changes should be helpful for single system maintainers also. It's still a big headache.

Once these changes are reviewed I will make sure the Documentation files are updated, but I expect some back and forth first.

Michael Rubin (3):
writeback: Creating /sys/kernel/mm/writeback/writeback
writeback: per bdi monitoring
writeback: tracking subsystems causing writeback

 drivers/base/node.c         |   14 +++++
 fs/buffer.c                 |    2 +-
 fs/fs-writeback.c           |   28 +++++++--
 fs/nilfs2/segment.c         |    4 +-
 fs/sync.c                   |    2 +-
 include/linux/backing-dev.h |    9 +++
 include/linux/mmzone.h      |    2 +
 include/linux/writeback.h   |   50 +++++++++++++++-
 mm/backing-dev.c            |  137 ++++++++++++++++++++++---------------------
 mm/mm_init.c                |  122 ++++++++++++++++++++++++++++++++++++++
 mm/page-writeback.c         |   18 ++++--
 mm/vmscan.c                 |    3 +-
 mm/vmstat.c                 |    2 +
 13 files changed, 311 insertions(+), 82 deletions(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Adding the /sys/kernel/mm/writeback/writeback file. It contains data to help developers and applications gain visibility into writeback behaviour.

    # cat /sys/kernel/mm/writeback/writeback
    pages_dirtied:    3747
    pages_cleaned:    3618
    dirty_threshold:  816673
    bg_threshold:     408336

The motivation of a sys file as opposed to debugfs is that applications that do not have permissions to mount file systems often need this data.

In order to track the "cleaned" and "dirtied" counts we added two vm_stat_items. Per memory node stats have been added also. So we can see per node granularity:

    # cat /sys/devices/system/node/node20/writebackstat
    Node 20 pages_writeback: 0 times
    Node 20 pages_dirtied: 0 times

---
 drivers/base/node.c    |   14 ++++++++++++++
 fs/nilfs2/segment.c    |    4 +++-
 include/linux/mmzone.h |    2 ++
 mm/mm_init.c           |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 mm/page-writeback.c    |    6 ++++--
 mm/vmstat.c            |    2 ++
 6 files changed, 73 insertions(+), 3 deletions(-)

index 2bdd8a9..b321d32 100644
@@ -160,6 +160,18 @@ static ssize_t node_read_numastat(struct sys_device * dev,
 }
 static SYSDEV_ATTR(numastat, S_IRUGO, node_read_numastat, NULL);
 
+static ssize_t node_read_writebackstat(struct sys_device *dev,
+				struct sysdev_attribute *attr, char *buf)
+{
+	int nid = dev->id;
+	return sprintf(buf,
+		"Node %d pages_writeback: %lu times\n"
+		"Node %d pages_dirtied: %lu times\n",
+		nid, node_page_state(nid, NR_PAGES_ENTERED_WRITEBACK),
+		nid, node_page_state(nid, NR_FILE_PAGES_DIRTIED));
+}
+static SYSDEV_ATTR(writebackstat, S_IRUGO, node_read_writebackstat, NULL);
+
 static ssize_t node_read_distance(struct sys_device * dev,
 			struct sysdev_attribute *attr, char * buf)
 {

@@ -243,6 +255,7 @@ int register_node(struct node *node, int num, struct node *parent)
 		sysdev_create_file(&node->sysdev, &attr_meminfo);
 		sysdev_create_file(&node->sysdev, &attr_numastat);
 		sysdev_create_file(&node->sysdev, &attr_distance);
+		sysdev_create_file(&node->sysdev, &attr_writebackstat);
 
 		scan_unevictable_register_node(node);
 

@@ -267,6 +280,7 @@ void unregister_node(struct node *node)
 	sysdev_remove_file(&node->sysdev, &attr_meminfo);
 	sysdev_remove_file(&node->sysdev, &attr_numastat);
 	sysdev_remove_file(&node->sysdev, &attr_distance);
+	sysdev_remove_file(&node->sysdev, &attr_writebackstat);
 
 	scan_unevictable_unregister_node(node);
 	hugetlb_unregister_node(node);		/* no-op, if memoryless node */

index c920164..84b0181 100644
@@ -1598,8 +1598,10 @@ nilfs_copy_replace_page_buffers(struct page *page, struct list_head *out)
 	} while (bh = bh->b_this_page, bh2 = bh2->b_this_page, bh != head);
 	kunmap_atomic(kaddr, KM_USER0);
 
-	if (!TestSetPageWriteback(clone_page))
+	if (!TestSetPageWriteback(clone_page)) {
 		inc_zone_page_state(clone_page, NR_WRITEBACK);
+		inc_zone_page_state(clone_page, NR_PAGES_ENTERED_WRITEBACK);
+	}
 	unlock_page(clone_page);
 
 	return 0;

index b4d109e..c0cd2bd 100644
@@ -112,6 +112,8 @@ enum zone_stat_item {
 	NUMA_LOCAL,		/* allocation from local node */
 	NUMA_OTHER,		/* allocation from other node */
 #endif
+	NR_PAGES_ENTERED_WRITEBACK, /* number of times pages enter writeback */
+	NR_FILE_PAGES_DIRTIED,    /* number of times pages get dirtied */
 	NR_VM_ZONE_STAT_ITEMS };
 
 /*

index bbd396a..4cea5c0 100644
@@ -1093,6 +1093,7 @@ void account_page_dirtied(struct page *page, struct address_space *mapping)
 {
 	if (mapping_cap_account_dirty(mapping)) {
 		__inc_zone_page_state(page, NR_FILE_DIRTY);
+		__inc_zone_page_state(page, NR_FILE_PAGES_DIRTIED);
 		__inc_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
 		task_dirty_inc(current);
 		task_io_account_write(PAGE_CACHE_SIZE);

@@ -1333,10 +1334,11 @@ int test_set_page_writeback(struct page *page)
 	} else {
 		ret = TestSetPageWriteback(page);
 	}
-	if (!ret)
+	if (!ret) {
 		inc_zone_page_state(page, NR_WRITEBACK);
+		inc_zone_page_state(page, NR_PAGES_ENTERED_WRITEBACK);
+	}
 	return ret;
-
 }
 EXPORT_SYMBOL(test_set_page_writeback);
 

index 7759941..e177a40 100644
@@ -740,6 +740,8 @@ static const char * const vmstat_text[] = {
 	"numa_local",
 	"numa_other",
 #endif
+	"nr_pages_entered_writeback",
+	"nr_file_pages_dirtied",
 
 #ifdef CONFIG_VM_EVENT_COUNTERS
 	"pgpgin",

1.7.0.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I'm fine with exposting this. but the interface is rather awkward. These kinds of multiple value per file interface require addition parsing and are a pain to extend. Please do something like

/proc/sys/vm/writeback/

pages_dirtied
pages_cleaned
dirty_threshold
background_threshold

where you can just read the value from the file.

Thanks for looking at this.

Cool. This is kind of funny. In the google tree I implemented this in the same multi-file-one-value-in-file manner. The debate on one file for all vs that style was heated. And I changed it before sending upstream. I really don't care either way. So I will just change the patch and move the values to that location

Do you mind explaining why something would go in /proc/ vs /sys? I thought the idea was to not put things in /proc anymore.

I wasn't excited about this section either. What does opencoded mean? Do you mean it should not be exposed to specific fs code?

mrubin
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

To allow users and applications to gain visibility into the writeback behaviour debugfs files are moved into /sys. bdi granularity of visibility is important to root cause both rogue user apps and/or kernel issues.

This patch converts bdi debug files of bdi into
/sys/block/<dev>/bdi/writeback.

    # cat /sys/class/block/sda/bdi/writeback
    BdiWriteback:             0 kB
    BdiReclaimable:          96 kB
    BdiDirtyThresh:     2117144 kB
    DirtyThresh:       12192420 kB
    BackgroundThresh:   1625656 kB
    WritebackThreads:         1
    WorkWaiting:              0
    InodesOnDirty:           37
    InodesOnIo:               0
    InodesOnMoreIo:           0
    wb_mask:                  1
    wb_cnt:                   1

Also it adds a sys file for the bdi state in /sys/block/<block>/bdi/state. The file will expose the state of the bdi in a comma separated list.

# cat /sys/class/block/sda/bdi/state
registered,sync_congested

TESTED:
Injected false states in the code stream to ensure the state file worked correctly. Then removed false states.

Performed IO on different disks to see the correct writeback values fluctuate and report what was expected.

---
 fs/fs-writeback.c           |   13 +++++
 include/linux/backing-dev.h |    1 +
 mm/backing-dev.c            |  126 +++++++++++++++++++------------------------
 3 files changed, 70 insertions(+), 70 deletions(-)

index 1d1088f..62a899e 100644
@@ -95,6 +95,19 @@ int writeback_in_progress(struct backing_dev_info *bdi)
 	return !list_empty(&bdi->work_list);
 }
 
+int writeback_work_waiting(struct backing_dev_info *bdi)
+{
+	struct bdi_work *work;
+	int nr_wb_work = 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(work, &bdi->work_list, list) {
+		nr_wb_work++;
+	}
+	rcu_read_unlock();
+	return nr_wb_work;
+}
+
 static void bdi_work_clear(struct bdi_work *work)
 {
 	clear_bit(WS_USED_B, &work->state);

index aee5f6c..e1fb11c 100644
@@ -255,6 +255,7 @@ extern struct backing_dev_info noop_backing_dev_info;
 void default_unplug_io_fn(struct backing_dev_info *bdi, struct page *page);
 
 int writeback_in_progress(struct backing_dev_info *bdi);
+int writeback_work_waiting(struct backing_dev_info *bdi);
 
 static inline int bdi_congested(struct backing_dev_info *bdi, int bdi_bits)
 {

1.7.0.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Adding stats to indicate which subsystem is causing writeback. Implemented as per cpu counters rather than tracepoints so we can gather statistics continuously and cheaply.

Knowing what is the source of writeback acitivity can help tune and debug applications.

The data is exported in bdi granularity at /sys/block/<dev>/bdi/writeback_stats.

    # cat /sys/block/sda/bdi/writeback_stats
    balance dirty pages                       0
    balance dirty pages waiting               0
    periodic writeback                    92024
    periodic writeback exited                 0
    laptop periodic                           0
    laptop or bg threshold                    0
    free more memory                          0
    try to free pages                       271
    syc_sync                                  6
    sync filesystem                           0

For the entire system you can find them at /sys/kernel/mm/writeback/stats.

    # cat /sys/kernel/mm/writeback/stats
    balance dirty pages                       0
    balance dirty pages waiting               0
    periodic writeback                    92892
    periodic writeback exited                13
    laptop periodic                           0
    laptop or bg threshold                    0
    free more memory                          0
    try to free pages                       271
    syc_sync                                  6
    sync filesystem                           0

---
 fs/buffer.c                 |    2 +-
 fs/fs-writeback.c           |   15 +++++---
 fs/sync.c                   |    2 +-
 include/linux/backing-dev.h |    8 ++++
 include/linux/writeback.h   |   50 +++++++++++++++++++++++++-
 mm/backing-dev.c            |   17 +++++++++
 mm/mm_init.c                |   82 ++++++++++++++++++++++++++++++++++++++++--
 mm/page-writeback.c         |   12 +++++--
 mm/vmscan.c                 |    3 +-
 9 files changed, 175 insertions(+), 16 deletions(-)

index d54812b..2a0ebe0 100644
@@ -288,7 +288,7 @@ static void free_more_memory(void)
 	struct zone *zone;
 	int nid;
 
-	wakeup_flusher_threads(1024);
+	wakeup_flusher_threads(1024, WB_STAT_FREE_MORE_MEM);
 	yield();
 
 	for_each_online_node(nid) {

index 62a899e..56331b0 100644
@@ -956,6 +956,7 @@ int bdi_writeback_task(struct bdi_writeback *wb)
 	long pages_written;
 
 	while (!kthread_should_stop()) {
+		bdi_writeback_stat_inc(wb->bdi, WB_STAT_PERIODIC);
 		pages_written = wb_do_writeback(wb, 0);
 
 		if (pages_written)

@@ -969,8 +970,11 @@ int bdi_writeback_task(struct bdi_writeback *wb)
 			 * recreated automatically.
 			 */
 			max_idle = max(5UL * 60 * HZ, wait_jiffies);
-			if (time_after(jiffies, max_idle + last_active))
+			if (time_after(jiffies, max_idle + last_active)) {
+				bdi_writeback_stat_inc(wb->bdi,
+						      WB_STAT_PERIODIC_EXIT);
 				break;
+			}
 		}
 
 		if (dirty_writeback_interval) {

@@ -994,7 +998,8 @@ int bdi_writeback_task(struct bdi_writeback *wb)
  * Schedule writeback for all backing devices. This does WB_SYNC_NONE
  * writeback, for integrity writeback see bdi_sync_writeback().
  */
-static void bdi_writeback_all(struct super_block *sb, long nr_pages)
+static void bdi_writeback_all(struct super_block *sb, long nr_pages,
+			      enum wb_stats stat)
 {
 	struct wb_writeback_args args = {
 		.sb		= sb,

@@ -1008,7 +1013,7 @@ static void bdi_writeback_all(struct super_block *sb, long nr_pages)
 	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) {
 		if (!bdi_has_dirty_io(bdi))
 			continue;
-
+		bdi_writeback_stat_inc(bdi, stat);
 		bdi_alloc_queue_work(bdi, &args);
 	}
 

@@ -1019,12 +1024,12 @@ static void bdi_writeback_all(struct super_block *sb, long nr_pages)
  * 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_stats stat)
 {
 	if (nr_pages == 0)
 		nr_pages = global_page_state(NR_FILE_DIRTY) +
 				global_page_state(NR_UNSTABLE_NFS);
-	bdi_writeback_all(NULL, nr_pages);
+	bdi_writeback_all(NULL, nr_pages, stat);
 }
 
 static noinline void block_dump___mark_inode_dirty(struct inode *inode)

index 15aa6f0..6059e83 100644
@@ -97,7 +97,7 @@ static void sync_filesystems(int wait)
  */
 SYSCALL_DEFINE0(sync)
 {
-	wakeup_flusher_threads(0);
+	wakeup_flusher_threads(0, WB_STAT_SYNC);
 	sync_filesystems(0);
 	sync_filesystems(1);
 	if (unlikely(laptop_mode))

index e1fb11c..dfa7c78 100644
@@ -72,6 +72,7 @@ struct backing_dev_info {
 	char *name;
 
 	struct percpu_counter bdi_stat[NR_BDI_STAT_ITEMS];
+	struct writeback_stats *wb_stat;
 
 	struct prop_local_percpu completions;
 	int dirty_exceeded;

@@ -184,6 +185,13 @@ static inline s64 bdi_stat_sum(struct backing_dev_info *bdi,
 	return sum;
 }
 
+static inline void bdi_writeback_stat_inc(struct backing_dev_info *bdi,
+					 enum wb_stats stat)
+{
+	if (bdi)
+		writeback_stats_inc(bdi->wb_stat, stat);
+}
+
 extern void bdi_writeout_inc(struct backing_dev_info *bdi);
 
 /*

index 9a89296..83efc39 100644
@@ -142,6 +142,13 @@ static ssize_t writeback_show(struct device *dev, struct device_attribute *attr,
 #undef K
 }
 
+static ssize_t writeback_stats_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct backing_dev_info *bdi = dev_get_drvdata(dev);
+	return writeback_stats_print(bdi->wb_stat, buf, PAGE_SIZE);
+}
+
 static ssize_t read_ahead_kb_store(struct device *dev,
 				  struct device_attribute *attr,
 				  const char *buf, size_t count)

@@ -213,6 +220,7 @@ BDI_SHOW(max_ratio, bdi->max_ratio)
 static struct device_attribute bdi_dev_attrs[] = {
 	__ATTR_RO(state),
 	__ATTR_RO(writeback),
+	__ATTR_RO(writeback_stats),
 	__ATTR_RW(read_ahead_kb),
 	__ATTR_RW(min_ratio),
 	__ATTR_RW(max_ratio),

@@ -673,11 +681,18 @@ int bdi_init(struct backing_dev_info *bdi)
 			goto err;
 	}
 
+	bdi->wb_stat = writeback_stats_alloc();
+	if (bdi->wb_stat == NULL) {
+		err = -ENOMEM;
+		goto err;
+	}
+
 	bdi->dirty_exceeded = 0;
 	err = prop_local_init_percpu(&bdi->completions);
 
 	if (err) {
 err:
+		writeback_stats_free(bdi->wb_stat);
 		while (i--)
 			percpu_counter_destroy(&bdi->bdi_stat[i]);
 	}

@@ -709,6 +724,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]);
 
+	writeback_stats_free(bdi->wb_stat);
+
 	prop_local_destroy_percpu(&bdi->completions);
 }
 EXPORT_SYMBOL(bdi_destroy);

index 4cea5c0..39d7ff2 100644
@@ -537,6 +537,7 @@ static void balance_dirty_pages(struct address_space *mapping,
 		 * up.
 		 */
 		if (bdi_nr_reclaimable > bdi_thresh) {
+			bdi_writeback_stat_inc(bdi, WB_STAT_BALANCE_DIRTY);
 			writeback_inodes_wbc(&wbc);
 			pages_written += write_chunk - wbc.nr_to_write;
 			get_dirty_limits(&background_thresh, &dirty_thresh,

@@ -567,6 +568,7 @@ static void balance_dirty_pages(struct address_space *mapping,
 			break;		/* We've done our duty */
 
 		__set_current_state(TASK_INTERRUPTIBLE);
+		bdi_writeback_stat_inc(bdi, WB_STAT_BALANCE_DIRTY_WAIT);
 		io_schedule_timeout(pause);
 
 		/*

@@ -596,8 +598,10 @@ static void balance_dirty_pages(struct address_space *mapping,
 	if ((laptop_mode && pages_written) ||
 	    (!laptop_mode && ((global_page_state(NR_FILE_DIRTY)
 			       + global_page_state(NR_UNSTABLE_NFS))
-					  > background_thresh)))
+					  > background_thresh))) {
+		bdi_writeback_stat_inc(bdi, WB_STAT_LAPTOP_BG);
 		bdi_start_writeback(bdi, NULL, 0);
+	}
 }
 
 void set_page_dirty_balance(struct page *page, int page_mkwrite)

@@ -700,14 +704,16 @@ void laptop_mode_timer_fn(unsigned long data)
 	struct request_queue *q = (struct request_queue *)data;
 	int nr_pages = global_page_state(NR_FILE_DIRTY) +
 		global_page_state(NR_UNSTABLE_NFS);
-
 	/*
 	 * We want to write everything out, not just down to the dirty
 	 * threshold
 	 */
 
-	if (bdi_has_dirty_io(&q->backing_dev_info))
+	if (bdi_has_dirty_io(&q->backing_dev_info)) {
+		writeback_stats_inc(q->backing_dev_info.wb_stat,
+				    WB_STAT_LAPTOP_TIMER);
 		bdi_start_writeback(&q->backing_dev_info, NULL, nr_pages);
+	}
 }
 
 /*

index 9c7e57c..ba6966e 100644
@@ -1838,7 +1838,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_STAT_TRY_TO_FREE_PAGES);
 			sc->may_writepage = 1;
 		}
 

1.7.0.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

That exports a lot of kernel internals in /sys, presumably read by some applications. What happens with the applications if the kernel internals ever change? Will the application break?

It would be bad to not be able to change the kernel because of such an interface.

-Andi

[email protected] -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Thanks for looking at this.

I agree. This would put the kernel in a box a bit. Some of them (sys_sync, periodic writeback, free_more_memory) I feel are generic enough concepts that with some rewording of the labels they could be exposed with no issue. "Balance_dirty_pages" is an example where that won't work.

Are there alternatives to this? Maybe tracepoints that are compiled to be on? A CONFIG_WRITEBACK_DEBUG that would expose this file?

Having this set of info readily available and collected makes debugging a lot easier. But I admit I am not sure the best way to expose them.

mrubin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Yes some rewording would be good.

The classic way is to put it into debugfs which has a appropiate disclaimer.

(although I fear we're weaning apps that depend on debugfs too The growing ftrace user space code seems to all depend on debugfs)

Maybe we just need a simpler writeback path that is not as complicated to debug.

-Andi

[email protected] -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I'm not sure we want to export statistics that represent internal implementation details into a fixed userspace API. Who, other than developers, are going to understand and be able to make use of this information?

FWIW, I've got to resend the writeback tracing patches to Jens that I have that give better visibility into the writeback behaviour. Perhaps those tracing events are a better basis for tracking down writeback problems - the bugs I found with the tracing could not have been found with these statistics...

That's really why I'm asking - if the stats are just there to help development and debugging, then I think that improving the writeback tracing is a better approach to improving visibility of writeback behaviour...

Cheers,

Dave.

Dave Chinner
[email protected]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Thanks for looking at this.

I think there are varying degrees of internal exposure on the patches.

This one seems to not expose any new internals. We already expose the concept of "dirty", "writeback" and thresholds in /proc/meminfo.

Looking at it again. I think this one is somewhat of a mixed bag. BDIReclaimable, BdiWriteback, and the dirty thresholds seems safe to export.While I agree the rest should stay in debugfs. Would that be amenable?

I definitely agree that this one is too revealing and needs to be redone. But I think we might want to add the details for concepts which we already expose.
The idea of a "periodic writeback" is already exposed in /proc/sys/vm/ and I don't see that changing in the kernel as a method to deal with buffered IO. Neither will sync. The laptop stuff and the names of "balance_dirty_pages" are bad, but maybe we can come up with something more high level. Like "writeback due to low memory"

Yeah I have been watching the tracing stuff you have posted and I think it will help. There were some other trace points I wanted to add to this patch but was waiting to learn from your submission on the best way to integrate them.

Maybe I should not have put all these patches in one series. The first one with the /sys/kernel/vm file is very useful for user space developers. System Administrators who are trying to classify IO problems often need to know if the disk is bad or if the buffered data is not even being written to disk over time.. Also at Google we tend to run our jobs with very little unused RAM. Pushing things close to their limits results in many surprises and writeback is often one of them. Knowing the thresholds and rate of dirty and cleaning of pages can help systems do the right thing.

mrubin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I don't see any probems with these stats - no matter the implementation, they'll still be relevant.

I'd much prefer all the bdi stats in the one spot. It's hard enough to find what you're looking for without splitting them into multiple locations.

The other thing to consider is that tracing requires debugfѕ to be mounted. Hence most kernels are going to have the debug stats available, anyway....

I don't see much value in exposing this information outside of development environments. I think it's much better to add trace points for events like this so that we do fine-grained analysis of when the events occur during problematic workloads....

I've got more work to do on them first.... :/

These stats aren't the place for observing that a disk is bad ;)

I think this can be obtained from the existing info in /proc/meminfo. I'm not saying the new stats aren't necessary, just that we already have the high level information available for this....

Yes, I hear this all the time from appliance developers that cache everything they need in userspace - they just want the kernel to stay out of the way and not use the unused RAM for caching stuff that doesn't matter to the application. Normally the issue is unbounded growth of the inode and dentry caches, but I can see how exceeding writeback limits can be just as much of a problem.

Cheers,

Dave.

Dave Chinner
[email protected]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Cool. I have a new patch I will send out tomorrow for these. They have been moved to /proc/sys/vm too as Christoph recommended. Makes more sense too.

Yeah I hear ya.

This thread has made me reconsider pursuing if there is a way that we can access debugfs safely in our environment. It would make things a lot easier.

They do help grant visibility in the whole stack of behaviour. Writeback has created a whole lot of confusion and time waste. I do agree with you that these should be folded into tracing infrastructure.

You hit the nail on the head. There's nothing like writing back logs to create latency spikes for direct IO traffic that make folks scratch their heads. In low memory environments this can get more confusing for a appliance developers trying to find out what happened after the fact.

Thanks again. I think (or hope) the next set of patches will be more applicable.

mrubin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>