[PATCH] laptop-mode: Make flushes per-device

[PATCH] laptop-mode: Make flushes per-device

[Matthew Garrett]

One of the features of laptop-mode is that it forces a writeout of dirty
pages if something else triggers a physical read or write from a device.
The current implementation flushes pages on all devices, rather than only
the one that triggered the flush. This patch alters the behaviour so that
only the recently accessed block device is flushed, preventing other
disks being spun up for no terribly good reason.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 block/blk-core.c            |    6 ++++-
 include/linux/backing-dev.h |    4 +++
 include/linux/writeback.h   |    5 +++-
 mm/page-writeback.c         |   44 ++++++++++++++++++++++++++----------------
 4 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 718897e..64e9b05 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -510,10 +510,14 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 		return NULL;
 	}
 
+	init_timer(&q->backing_dev_info.laptop_mode_wb_timer);
+	setup_timer(&q->backing_dev_info.laptop_mode_wb_timer,
+		    laptop_mode_timer_fn, (unsigned long) q);
 	init_timer(&q->unplug_timer);
 	setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
 	INIT_LIST_HEAD(&q->timeout_list);
 	INIT_WORK(&q->unplug_work, blk_unplug_work);
+	INIT_WORK(&q->backing_dev_info.laptop_mode_wb_work, laptop_mode_sync);
 
 	kobject_init(&q->kobj, &blk_queue_ktype);
 
@@ -2109,7 +2113,7 @@ static void blk_finish_request(struct request *req, int error)
 	BUG_ON(blk_queued_rq(req));
 
 	if (unlikely(laptop_mode) && blk_fs_request(req))
-		laptop_io_completion();
+		laptop_io_completion(req);
 
 	blk_delete_timer(req);
 
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index fcbc26a..98fed25 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/fs.h>
 #include <linux/sched.h>
+#include <linux/timer.h>
 #include <linux/writeback.h>
 #include <asm/atomic.h>
 
@@ -88,6 +89,9 @@ struct backing_dev_info {
 
 	struct device *dev;
 
+	struct timer_list laptop_mode_wb_timer;
+	struct work_struct laptop_mode_wb_work;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debug_dir;
 	struct dentry *debug_stats;
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 705f01f..7fdadb1 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -6,6 +6,7 @@
 
 #include <linux/sched.h>
 #include <linux/fs.h>
+#include <linux/blkdev.h>
 
 struct backing_dev_info;
 
@@ -93,8 +94,10 @@ static inline void inode_sync_wait(struct inode *inode)
 /*
  * mm/page-writeback.c
  */
-void laptop_io_completion(void);
+void laptop_io_completion(struct request *req);
 void laptop_sync_completion(void);
+void laptop_mode_sync(struct work_struct *work);
+void laptop_mode_timer_fn(unsigned long data);
 void throttle_vm_writeout(gfp_t gfp_mask);
 
 /* These are exported to sysctl. */
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b19943..9b62ebe 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -683,10 +683,6 @@ void throttle_vm_writeout(gfp_t gfp_mask)
         }
 }
 
-static void laptop_timer_fn(unsigned long unused);
-
-static DEFINE_TIMER(laptop_mode_wb_timer, laptop_timer_fn, 0, 0);
-
 /*
  * sysctl handler for /proc/sys/vm/dirty_writeback_centisecs
  */
@@ -697,21 +693,27 @@ int dirty_writeback_centisecs_handler(ctl_table *table, int write,
 	return 0;
 }
 
-static void do_laptop_sync(struct work_struct *work)
+void laptop_mode_sync(struct work_struct *work)
 {
-	wakeup_flusher_threads(0);
-	kfree(work);
+	struct backing_dev_info *bdi =
+		container_of(work, struct backing_dev_info,
+			     laptop_mode_wb_work);
+	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(bdi))
+		bdi_start_writeback(bdi, NULL, nr_pages);
 }
 
-static void laptop_timer_fn(unsigned long unused)
+void laptop_mode_timer_fn(unsigned long data)
 {
-	struct work_struct *work;
+	struct request_queue *q = (struct request_queue *)data;
 
-	work = kmalloc(sizeof(*work), GFP_ATOMIC);
-	if (work) {
-		INIT_WORK(work, do_laptop_sync);
-		schedule_work(work);
-	}
+	schedule_work(&q->backing_dev_info.laptop_mode_wb_work);
 }
 
 /*
@@ -719,9 +721,10 @@ static void laptop_timer_fn(unsigned long unused)
  * of all dirty data a few seconds from now.  If the flush is already scheduled
  * then push it back - the user is still using the disk.
  */
-void laptop_io_completion(void)
+void laptop_io_completion(struct request *req)
 {
-	mod_timer(&laptop_mode_wb_timer, jiffies + laptop_mode);
+	mod_timer(&req->q->backing_dev_info.laptop_mode_wb_timer,
+		  jiffies + laptop_mode);
 }
 
 /*
@@ -731,7 +734,14 @@ void laptop_io_completion(void)
  */
 void laptop_sync_completion(void)
 {
-	del_timer(&laptop_mode_wb_timer);
+	struct backing_dev_info *bdi;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list)
+		del_timer(&bdi->laptop_mode_wb_timer);
+
+	rcu_read_unlock();
 }
 
 /*
-- 
1.6.5.2

Re: [PATCH] laptop-mode: Make flushes per-device

[Matthew Garrett]

Sorry, I seem to have screwed up the includes somehow when I rebased 
this. I'm now trapped in a maze of twisty interdependencies, all 
different.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH] laptop-mode: Make flushes per-device

[Matthew Garrett]

One of the features of laptop-mode is that it forces a writeout of dirty
pages if something else triggers a physical read or write from a device.
The current implementation flushes pages on all devices, rather than only
the one that triggered the flush. This patch alters the behaviour so that
only the recently accessed block device is flushed, preventing other
disks being spun up for no terribly good reason.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---

The forward struct declaration in writeback.h seems messy, but I'm not
sure there's a cleaner way to do this. I'm also still woefully
unfamiliar with the block layer - I /think/ this does the right thing,
but some review would be nice.

 block/blk-core.c            |    6 ++++-
 include/linux/backing-dev.h |    4 +++
 include/linux/writeback.h   |    5 +++-
 mm/page-writeback.c         |   44 ++++++++++++++++++++++++++----------------
 4 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 718897e..64e9b05 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -510,10 +510,14 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 		return NULL;
 	}
 
+	init_timer(&q->backing_dev_info.laptop_mode_wb_timer);
+	setup_timer(&q->backing_dev_info.laptop_mode_wb_timer,
+		    laptop_mode_timer_fn, (unsigned long) q);
 	init_timer(&q->unplug_timer);
 	setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
 	INIT_LIST_HEAD(&q->timeout_list);
 	INIT_WORK(&q->unplug_work, blk_unplug_work);
+	INIT_WORK(&q->backing_dev_info.laptop_mode_wb_work, laptop_mode_sync);
 
 	kobject_init(&q->kobj, &blk_queue_ktype);
 
@@ -2109,7 +2113,7 @@ static void blk_finish_request(struct request *req, int error)
 	BUG_ON(blk_queued_rq(req));
 
 	if (unlikely(laptop_mode) && blk_fs_request(req))
-		laptop_io_completion();
+		laptop_io_completion(req);
 
 	blk_delete_timer(req);
 
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index fcbc26a..98fed25 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/fs.h>
 #include <linux/sched.h>
+#include <linux/timer.h>
 #include <linux/writeback.h>
 #include <asm/atomic.h>
 
@@ -88,6 +89,9 @@ struct backing_dev_info {
 
 	struct device *dev;
 
+	struct timer_list laptop_mode_wb_timer;
+	struct work_struct laptop_mode_wb_work;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debug_dir;
 	struct dentry *debug_stats;
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 705f01f..affcdb2 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -7,6 +7,7 @@
 #include <linux/sched.h>
 #include <linux/fs.h>
 
+struct request;
 struct backing_dev_info;
 
 extern spinlock_t inode_lock;
@@ -93,8 +94,10 @@ static inline void inode_sync_wait(struct inode *inode)
 /*
  * mm/page-writeback.c
  */
-void laptop_io_completion(void);
+void laptop_io_completion(struct request *req);
 void laptop_sync_completion(void);
+void laptop_mode_sync(struct work_struct *work);
+void laptop_mode_timer_fn(unsigned long data);
 void throttle_vm_writeout(gfp_t gfp_mask);
 
 /* These are exported to sysctl. */
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b19943..9b62ebe 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -683,10 +683,6 @@ void throttle_vm_writeout(gfp_t gfp_mask)
         }
 }
 
-static void laptop_timer_fn(unsigned long unused);
-
-static DEFINE_TIMER(laptop_mode_wb_timer, laptop_timer_fn, 0, 0);
-
 /*
  * sysctl handler for /proc/sys/vm/dirty_writeback_centisecs
  */
@@ -697,21 +693,27 @@ int dirty_writeback_centisecs_handler(ctl_table *table, int write,
 	return 0;
 }
 
-static void do_laptop_sync(struct work_struct *work)
+void laptop_mode_sync(struct work_struct *work)
 {
-	wakeup_flusher_threads(0);
-	kfree(work);
+	struct backing_dev_info *bdi =
+		container_of(work, struct backing_dev_info,
+			     laptop_mode_wb_work);
+	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(bdi))
+		bdi_start_writeback(bdi, NULL, nr_pages);
 }
 
-static void laptop_timer_fn(unsigned long unused)
+void laptop_mode_timer_fn(unsigned long data)
 {
-	struct work_struct *work;
+	struct request_queue *q = (struct request_queue *)data;
 
-	work = kmalloc(sizeof(*work), GFP_ATOMIC);
-	if (work) {
-		INIT_WORK(work, do_laptop_sync);
-		schedule_work(work);
-	}
+	schedule_work(&q->backing_dev_info.laptop_mode_wb_work);
 }
 
 /*
@@ -719,9 +721,10 @@ static void laptop_timer_fn(unsigned long unused)
  * of all dirty data a few seconds from now.  If the flush is already scheduled
  * then push it back - the user is still using the disk.
  */
-void laptop_io_completion(void)
+void laptop_io_completion(struct request *req)
 {
-	mod_timer(&laptop_mode_wb_timer, jiffies + laptop_mode);
+	mod_timer(&req->q->backing_dev_info.laptop_mode_wb_timer,
+		  jiffies + laptop_mode);
 }
 
 /*
@@ -731,7 +734,14 @@ void laptop_io_completion(void)
  */
 void laptop_sync_completion(void)
 {
-	del_timer(&laptop_mode_wb_timer);
+	struct backing_dev_info *bdi;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list)
+		del_timer(&bdi->laptop_mode_wb_timer);
+
+	rcu_read_unlock();
 }
 
 /*
-- 
1.6.5.2

Re: [PATCH] laptop-mode: Make flushes per-device

[Jens Axboe]

On Fri, Dec 11 2009, Matthew Garrett wrote:
> One of the features of laptop-mode is that it forces a writeout of dirty
> pages if something else triggers a physical read or write from a device.
> The current implementation flushes pages on all devices, rather than only
> the one that triggered the flush. This patch alters the behaviour so that
> only the recently accessed block device is flushed, preventing other
> disks being spun up for no terribly good reason.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
> 
> The forward struct declaration in writeback.h seems messy, but I'm not
> sure there's a cleaner way to do this. I'm also still woefully
> unfamiliar with the block layer - I /think/ this does the right thing,
> but some review would be nice.

That's because it is messy, why are you passing the request in? It
would be a lot more sane to pass in the queue, better still the backing
device.

What guarentees that the timer isn't running when the bdi goes away?

-- 
Jens Axboe

Re: [PATCH] laptop-mode: Make flushes per-device

[Matthew Garrett]

On Sat, Dec 12, 2009 at 01:24:43PM +0100, Jens Axboe wrote:

> That's because it is messy, why are you passing the request in? It
> would be a lot more sane to pass in the queue, better still the backing
> device.

It seemed conceptually cleaner for the completion to refer to the 
request rather than anything else, but that's easily fixed.

> What guarentees that the timer isn't running when the bdi goes away?

Good point. I'll fix these and resend.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] laptop-mode: Make flushes per-device

[Jens Axboe]

On Mon, Dec 14 2009, Matthew Garrett wrote:
> On Sat, Dec 12, 2009 at 01:24:43PM +0100, Jens Axboe wrote:
> 
> > That's because it is messy, why are you passing the request in? It
> > would be a lot more sane to pass in the queue, better still the backing
> > device.
> 
> It seemed conceptually cleaner for the completion to refer to the 
> request rather than anything else, but that's easily fixed.

It's rather a major layering violation, I would claim... Passing in the
bdi is design wise the cleanest.

> > What guarentees that the timer isn't running when the bdi goes away?
> 
> Good point. I'll fix these and resend.

Thanks! As you can tell, laptop mode was pretty much for a laptop with a
single drive back when it was created. It definitely seems like a good
idea to make it per-drive granular now.

-- 
Jens Axboe

[PATCH v2] laptop-mode: Make flushes per-device

[Matthew Garrett]

One of the features of laptop-mode is that it forces a writeout of dirty
pages if something else triggers a physical read or write from a device.
The current implementation flushes pages on all devices, rather than only
the one that triggered the flush. This patch alters the behaviour so that
only the recently accessed block device is flushed, preventing other
disks being spun up for no terribly good reason.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---

Changes the completion call to take a BDI rather than a request, and make
sure that there's a del_timer_sync in blk_cleanup_queue().

 block/blk-core.c            |    6 +++++-
 include/linux/backing-dev.h |    4 ++++
 include/linux/writeback.h   |    4 +++-
 mm/page-writeback.c         |   43 ++++++++++++++++++++++++++-----------------
 4 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 718897e..e293862 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -450,6 +450,7 @@ void blk_cleanup_queue(struct request_queue *q)
 	 */
 	blk_sync_queue(q);
 
+	del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
 	mutex_lock(&q->sysfs_lock);
 	queue_flag_set_unlocked(QUEUE_FLAG_DEAD, q);
 	mutex_unlock(&q->sysfs_lock);
@@ -510,10 +511,13 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 		return NULL;
 	}
 
+	setup_timer(&q->backing_dev_info.laptop_mode_wb_timer,
+		    laptop_mode_timer_fn, (unsigned long) q);
 	init_timer(&q->unplug_timer);
 	setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
 	INIT_LIST_HEAD(&q->timeout_list);
 	INIT_WORK(&q->unplug_work, blk_unplug_work);
+	INIT_WORK(&q->backing_dev_info.laptop_mode_wb_work, laptop_mode_sync);
 
 	kobject_init(&q->kobj, &blk_queue_ktype);
 
@@ -2109,7 +2113,7 @@ static void blk_finish_request(struct request *req, int error)
 	BUG_ON(blk_queued_rq(req));
 
 	if (unlikely(laptop_mode) && blk_fs_request(req))
-		laptop_io_completion();
+		laptop_io_completion(&req->q->backing_dev_info);
 
 	blk_delete_timer(req);
 
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index fcbc26a..98fed25 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/fs.h>
 #include <linux/sched.h>
+#include <linux/timer.h>
 #include <linux/writeback.h>
 #include <asm/atomic.h>
 
@@ -88,6 +89,9 @@ struct backing_dev_info {
 
 	struct device *dev;
 
+	struct timer_list laptop_mode_wb_timer;
+	struct work_struct laptop_mode_wb_work;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debug_dir;
 	struct dentry *debug_stats;
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 705f01f..c2ca22c 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -93,8 +93,10 @@ static inline void inode_sync_wait(struct inode *inode)
 /*
  * mm/page-writeback.c
  */
-void laptop_io_completion(void);
+void laptop_io_completion(struct backing_dev_info *info);
 void laptop_sync_completion(void);
+void laptop_mode_sync(struct work_struct *work);
+void laptop_mode_timer_fn(unsigned long data);
 void throttle_vm_writeout(gfp_t gfp_mask);
 
 /* These are exported to sysctl. */
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b19943..193ca76 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -683,10 +683,6 @@ void throttle_vm_writeout(gfp_t gfp_mask)
         }
 }
 
-static void laptop_timer_fn(unsigned long unused);
-
-static DEFINE_TIMER(laptop_mode_wb_timer, laptop_timer_fn, 0, 0);
-
 /*
  * sysctl handler for /proc/sys/vm/dirty_writeback_centisecs
  */
@@ -697,21 +693,27 @@ int dirty_writeback_centisecs_handler(ctl_table *table, int write,
 	return 0;
 }
 
-static void do_laptop_sync(struct work_struct *work)
+void laptop_mode_sync(struct work_struct *work)
 {
-	wakeup_flusher_threads(0);
-	kfree(work);
+	struct backing_dev_info *bdi =
+		container_of(work, struct backing_dev_info,
+			     laptop_mode_wb_work);
+	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(bdi))
+		bdi_start_writeback(bdi, NULL, nr_pages);
 }
 
-static void laptop_timer_fn(unsigned long unused)
+void laptop_mode_timer_fn(unsigned long data)
 {
-	struct work_struct *work;
+	struct request_queue *q = (struct request_queue *)data;
 
-	work = kmalloc(sizeof(*work), GFP_ATOMIC);
-	if (work) {
-		INIT_WORK(work, do_laptop_sync);
-		schedule_work(work);
-	}
+	schedule_work(&q->backing_dev_info.laptop_mode_wb_work);
 }
 
 /*
@@ -719,9 +721,9 @@ static void laptop_timer_fn(unsigned long unused)
  * of all dirty data a few seconds from now.  If the flush is already scheduled
  * then push it back - the user is still using the disk.
  */
-void laptop_io_completion(void)
+void laptop_io_completion(struct backing_dev_info *info)
 {
-	mod_timer(&laptop_mode_wb_timer, jiffies + laptop_mode);
+	mod_timer(&info->laptop_mode_wb_timer, jiffies + laptop_mode);
 }
 
 /*
@@ -731,7 +733,14 @@ void laptop_io_completion(void)
  */
 void laptop_sync_completion(void)
 {
-	del_timer(&laptop_mode_wb_timer);
+	struct backing_dev_info *bdi;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list)
+		del_timer(&bdi->laptop_mode_wb_timer);
+
+	rcu_read_unlock();
 }
 
 /*
-- 
1.6.5.2

Re: [PATCH v2] laptop-mode: Make flushes per-device

[Jens Axboe]

On Mon, Dec 14 2009, Matthew Garrett wrote:
> One of the features of laptop-mode is that it forces a writeout of dirty
> pages if something else triggers a physical read or write from a device.
> The current implementation flushes pages on all devices, rather than only
> the one that triggered the flush. This patch alters the behaviour so that
> only the recently accessed block device is flushed, preventing other
> disks being spun up for no terribly good reason.

A few comments... Perhaps the timer deletion should go into the backing,
since that is where it's sitting?

Also, I think it would be cleaner to queue the flush work from the timer
on the per-bdi thread, instead of having a work struct allocated and
using that work item to simply call bdi writeback instead.

-- 
Jens Axboe

Re: [PATCH v2] laptop-mode: Make flushes per-device

[Matthew Garrett]

On Mon, Dec 14, 2009 at 08:58:06PM +0100, Jens Axboe wrote:
> On Mon, Dec 14 2009, Matthew Garrett wrote:
> > One of the features of laptop-mode is that it forces a writeout of dirty
> > pages if something else triggers a physical read or write from a device.
> > The current implementation flushes pages on all devices, rather than only
> > the one that triggered the flush. This patch alters the behaviour so that
> > only the recently accessed block device is flushed, preventing other
> > disks being spun up for no terribly good reason.
> 
> A few comments... Perhaps the timer deletion should go into the backing,
> since that is where it's sitting?

That was for symmetry with the setup, but I'm not married to it.

> Also, I think it would be cleaner to queue the flush work from the timer
> on the per-bdi thread, instead of having a work struct allocated and
> using that work item to simply call bdi writeback instead.

I had a vague recollection of context awkwardness, but I may have been 
wrong there. I'll try that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH v3] laptop-mode: Make flushes per-device

[Matthew Garrett]

One of the features of laptop-mode is that it forces a writeout of dirty
pages if something else triggers a physical read or write from a device.
The current implementation flushes pages on all devices, rather than only
the one that triggered the flush. This patch alters the behaviour so that
only the recently accessed block device is flushed, preventing other
disks being spun up for no terribly good reason.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---

This removes the work struct and triggers the writeback directly from the
timer.

 block/blk-core.c            |    5 ++++-
 include/linux/backing-dev.h |    3 +++
 include/linux/writeback.h   |    4 +++-
 mm/page-writeback.c         |   39 ++++++++++++++++++++-------------------
 4 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 718897e..6cddae7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -450,6 +450,7 @@ void blk_cleanup_queue(struct request_queue *q)
 	 */
 	blk_sync_queue(q);
 
+	del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
 	mutex_lock(&q->sysfs_lock);
 	queue_flag_set_unlocked(QUEUE_FLAG_DEAD, q);
 	mutex_unlock(&q->sysfs_lock);
@@ -510,6 +511,8 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 		return NULL;
 	}
 
+	setup_timer(&q->backing_dev_info.laptop_mode_wb_timer,
+		    laptop_mode_timer_fn, (unsigned long) q);
 	init_timer(&q->unplug_timer);
 	setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
 	INIT_LIST_HEAD(&q->timeout_list);
@@ -2109,7 +2112,7 @@ static void blk_finish_request(struct request *req, int error)
 	BUG_ON(blk_queued_rq(req));
 
 	if (unlikely(laptop_mode) && blk_fs_request(req))
-		laptop_io_completion();
+		laptop_io_completion(&req->q->backing_dev_info);
 
 	blk_delete_timer(req);
 
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index fcbc26a..2742e1a 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/fs.h>
 #include <linux/sched.h>
+#include <linux/timer.h>
 #include <linux/writeback.h>
 #include <asm/atomic.h>
 
@@ -88,6 +89,8 @@ struct backing_dev_info {
 
 	struct device *dev;
 
+	struct timer_list laptop_mode_wb_timer;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debug_dir;
 	struct dentry *debug_stats;
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 705f01f..c2ca22c 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -93,8 +93,10 @@ static inline void inode_sync_wait(struct inode *inode)
 /*
  * mm/page-writeback.c
  */
-void laptop_io_completion(void);
+void laptop_io_completion(struct backing_dev_info *info);
 void laptop_sync_completion(void);
+void laptop_mode_sync(struct work_struct *work);
+void laptop_mode_timer_fn(unsigned long data);
 void throttle_vm_writeout(gfp_t gfp_mask);
 
 /* These are exported to sysctl. */
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b19943..d0f2b37 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -683,10 +683,6 @@ void throttle_vm_writeout(gfp_t gfp_mask)
         }
 }
 
-static void laptop_timer_fn(unsigned long unused);
-
-static DEFINE_TIMER(laptop_mode_wb_timer, laptop_timer_fn, 0, 0);
-
 /*
  * sysctl handler for /proc/sys/vm/dirty_writeback_centisecs
  */
@@ -697,21 +693,19 @@ int dirty_writeback_centisecs_handler(ctl_table *table, int write,
 	return 0;
 }
 
-static void do_laptop_sync(struct work_struct *work)
+void laptop_mode_timer_fn(unsigned long data)
 {
-	wakeup_flusher_threads(0);
-	kfree(work);
-}
+	struct request_queue *q = (struct request_queue *)data;
+	int nr_pages = global_page_state(NR_FILE_DIRTY) +
+		global_page_state(NR_UNSTABLE_NFS);
 
-static void laptop_timer_fn(unsigned long unused)
-{
-	struct work_struct *work;
+	/*
+	 * We want to write everything out, not just down to the dirty
+	 * threshold
+	 */
 
-	work = kmalloc(sizeof(*work), GFP_ATOMIC);
-	if (work) {
-		INIT_WORK(work, do_laptop_sync);
-		schedule_work(work);
-	}
+	if (bdi_has_dirty_io(&q->backing_dev_info))
+		bdi_start_writeback(&q->backing_dev_info, NULL, nr_pages);
 }
 
 /*
@@ -719,9 +713,9 @@ static void laptop_timer_fn(unsigned long unused)
  * of all dirty data a few seconds from now.  If the flush is already scheduled
  * then push it back - the user is still using the disk.
  */
-void laptop_io_completion(void)
+void laptop_io_completion(struct backing_dev_info *info)
 {
-	mod_timer(&laptop_mode_wb_timer, jiffies + laptop_mode);
+	mod_timer(&info->laptop_mode_wb_timer, jiffies + laptop_mode);
 }
 
 /*
@@ -731,7 +725,14 @@ void laptop_io_completion(void)
  */
 void laptop_sync_completion(void)
 {
-	del_timer(&laptop_mode_wb_timer);
+	struct backing_dev_info *bdi;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(bdi, &bdi_list, bdi_list)
+		del_timer(&bdi->laptop_mode_wb_timer);
+
+	rcu_read_unlock();
 }
 
 /*
-- 
1.6.5.2

Re: [PATCH v3] laptop-mode: Make flushes per-device

[Matthew Garrett]

On Tue, Dec 15, 2009 at 01:21:35PM -0500, Matthew Garrett wrote:
> One of the features of laptop-mode is that it forces a writeout of dirty
> pages if something else triggers a physical read or write from a device.
> The current implementation flushes pages on all devices, rather than only
> the one that triggered the flush. This patch alters the behaviour so that
> only the recently accessed block device is flushed, preventing other
> disks being spun up for no terribly good reason.

Hi Jens,

Can you remember if there were any more issues with this, or can this 
version be merged now?

Thanks,
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3] laptop-mode: Make flushes per-device

[Jens Axboe]

On Sat, Apr 03 2010, Matthew Garrett wrote:
> On Tue, Dec 15, 2009 at 01:21:35PM -0500, Matthew Garrett wrote:
> > One of the features of laptop-mode is that it forces a writeout of dirty
> > pages if something else triggers a physical read or write from a device.
> > The current implementation flushes pages on all devices, rather than only
> > the one that triggered the flush. This patch alters the behaviour so that
> > only the recently accessed block device is flushed, preventing other
> > disks being spun up for no terribly good reason.
> 
> Hi Jens,
> 
> Can you remember if there were any more issues with this, or can this 
> version be merged now?

I think it's fine to merge, I'll queue it up for 2.6.35.

-- 
Jens Axboe