[PATCH, RFC] simplify writeback thread creation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Currently the per-bdi writeback thread is only created when there is
dirty any dirty data on the BDI, and it lazy exists when it's been
unused for some time.

This leads to some very complex code, and the need to keep a forker
thread around.

This patch removes all this code and simply creates the thread as part
of the bdi registration.  The downside is that we use up ressoures
for possible unused devices, although that overhead is rather low,
with 8k kernel stack size on x86 and few other, even smaller ressources.

If the overhead is still considered too much I can look into starting
the thread explicitly instead of as part of the bdi registration, but
that will require a bit of code complexity, too.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>

---

 fs/fs-writeback.c                |   60 ---------
 include/linux/backing-dev.h      |   14 --
 include/linux/writeback.h        |    1 
 include/trace/events/writeback.h |    3 
 mm/backing-dev.c                 |  252 ++++-----------------------------------
 5 files changed, 33 insertions(+), 297 deletions(-)

Index: linux-2.6/fs/fs-writeback.c
===================================================================
--- linux-2.6.orig/fs/fs-writeback.c	2010-07-07 11:26:00.000000000 -0700
+++ linux-2.6/fs/fs-writeback.c	2010-07-07 11:36:20.196688282 -0700
@@ -80,19 +80,7 @@ static void bdi_queue_work(struct backin
 	list_add_tail(&work->list, &bdi->work_list);
 	spin_unlock(&bdi->wb_lock);
 
-	/*
-	 * If the default thread isn't there, make sure we add it. When
-	 * it gets created and wakes up, we'll run this work.
-	 */
-	if (unlikely(!bdi->wb.task)) {
-		trace_writeback_nothread(bdi, work);
-		wake_up_process(default_backing_dev_info.wb.task);
-	} else {
-		struct bdi_writeback *wb = &bdi->wb;
-
-		if (wb->task)
-			wake_up_process(wb->task);
-	}
+	wake_up_process(bdi->wb.task);
 }
 
 static void
@@ -107,10 +95,8 @@ __bdi_start_writeback(struct backing_dev
 	 */
 	work = kzalloc(sizeof(*work), GFP_ATOMIC);
 	if (!work) {
-		if (bdi->wb.task) {
-			trace_writeback_nowork(bdi);
-			wake_up_process(bdi->wb.task);
-		}
+		trace_writeback_nowork(bdi);
+		wake_up_process(bdi->wb.task);
 		return;
 	}
 
@@ -756,7 +742,7 @@ static long wb_check_old_data_flush(stru
 /*
  * Retrieve work items and do the writeback they describe
  */
-long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
+static long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
 {
 	struct backing_dev_info *bdi = wb->bdi;
 	struct wb_writeback_work *work;
@@ -800,17 +786,8 @@ int bdi_writeback_thread(void *data)
 {
 	struct bdi_writeback *wb = data;
 	struct backing_dev_info *bdi = wb->bdi;
-	unsigned long last_active = jiffies;
-	unsigned long wait_jiffies = -1UL;
 	long pages_written;
 
-	/*
-	 * Add us to the active bdi_list
-	 */
-	spin_lock_bh(&bdi_lock);
-	list_add_rcu(&bdi->bdi_list, &bdi_list);
-	spin_unlock_bh(&bdi_lock);
-
 	current->flags |= PF_FLUSHER | PF_SWAPWRITE;
 	set_freezable();
 
@@ -819,38 +796,14 @@ int bdi_writeback_thread(void *data)
 	 */
 	set_user_nice(current, 0);
 
-	/*
-	 * Clear pending bit and wakeup anybody waiting to tear us down
-	 */
-	clear_bit(BDI_pending, &bdi->state);
-	smp_mb__after_clear_bit();
-	wake_up_bit(&bdi->state, BDI_pending);
-
-	trace_writeback_thread_start(bdi);
-
 	while (!kthread_should_stop()) {
 		pages_written = wb_do_writeback(wb, 0);
 
 		trace_writeback_pages_written(pages_written);
 
-		if (pages_written)
-			last_active = jiffies;
-		else if (wait_jiffies != -1UL) {
-			unsigned long max_idle;
-
-			/*
-			 * Longest period of inactivity that we tolerate. If we
-			 * see dirty data again later, the task will get
-			 * recreated automatically.
-			 */
-			max_idle = max(5UL * 60 * HZ, wait_jiffies);
-			if (time_after(jiffies, max_idle + last_active))
-				break;
-		}
-
 		if (dirty_writeback_interval) {
-			wait_jiffies = msecs_to_jiffies(dirty_writeback_interval * 10);
-			schedule_timeout_interruptible(wait_jiffies);
+			schedule_timeout_interruptible(msecs_to_jiffies(
+					dirty_writeback_interval * 10));
 		} else {
 			set_current_state(TASK_INTERRUPTIBLE);
 			if (list_empty_careful(&wb->bdi->work_list) &&
@@ -871,7 +824,6 @@ int bdi_writeback_thread(void *data)
 	if (!list_empty(&bdi->work_list))
 		wb_do_writeback(wb, 1);
 
-	trace_writeback_thread_stop(bdi);
 	return 0;
 }
 
Index: linux-2.6/include/linux/backing-dev.h
===================================================================
--- linux-2.6.orig/include/linux/backing-dev.h	2010-07-07 11:33:00.427687937 -0700
+++ linux-2.6/include/linux/backing-dev.h	2010-07-07 11:38:34.224687973 -0700
@@ -26,8 +26,6 @@ struct dentry;
  * Bits in backing_dev_info.state
  */
 enum bdi_state {
-	BDI_pending,		/* On its way to being activated */
-	BDI_wb_alloc,		/* Default embedded wb allocated */
 	BDI_async_congested,	/* The async (write) queue is getting full */
 	BDI_sync_congested,	/* The sync queue is getting full */
 	BDI_registered,		/* bdi_register() was done */
@@ -58,7 +56,6 @@ struct bdi_writeback {
 
 struct backing_dev_info {
 	struct list_head bdi_list;
-	struct rcu_head rcu_head;
 	unsigned long ra_pages;	/* max readahead in PAGE_CACHE_SIZE units */
 	unsigned long state;	/* Always use atomic bitops on this */
 	unsigned int capabilities; /* Device capabilities */
@@ -306,11 +303,6 @@ static inline bool bdi_cap_swap_backed(s
 	return bdi->capabilities & BDI_CAP_SWAP_BACKED;
 }
 
-static inline bool bdi_cap_flush_forker(struct backing_dev_info *bdi)
-{
-	return bdi == &default_backing_dev_info;
-}
-
 static inline bool mapping_cap_writeback_dirty(struct address_space *mapping)
 {
 	return bdi_cap_writeback_dirty(mapping->backing_dev_info);
@@ -326,12 +318,6 @@ static inline bool mapping_cap_swap_back
 	return bdi_cap_swap_backed(mapping->backing_dev_info);
 }
 
-static inline int bdi_sched_wait(void *word)
-{
-	schedule();
-	return 0;
-}
-
 static inline void blk_run_backing_dev(struct backing_dev_info *bdi,
 				       struct page *page)
 {
Index: linux-2.6/include/linux/writeback.h
===================================================================
--- linux-2.6.orig/include/linux/writeback.h	2010-07-07 10:28:22.000000000 -0700
+++ linux-2.6/include/linux/writeback.h	2010-07-07 11:33:12.144687938 -0700
@@ -64,7 +64,6 @@ int writeback_inodes_sb_if_idle(struct s
 void sync_inodes_sb(struct super_block *);
 void writeback_inodes_wb(struct bdi_writeback *wb,
 		struct writeback_control *wbc);
-long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
 void wakeup_flusher_threads(long nr_pages);
 
 /* writeback.h requires fs.h; it, too, is not included from here. */
Index: linux-2.6/mm/backing-dev.c
===================================================================
--- linux-2.6.orig/mm/backing-dev.c	2010-07-07 11:33:00.437687937 -0700
+++ linux-2.6/mm/backing-dev.c	2010-07-07 12:02:28.694687934 -0700
@@ -36,13 +36,10 @@ EXPORT_SYMBOL_GPL(noop_backing_dev_info)
 static struct class *bdi_class;
 
 /*
- * bdi_lock protects updates to bdi_list and bdi_pending_list, as well as
- * reader side protection for bdi_pending_list. bdi_list has RCU reader side
- * locking.
+ * bdi_lock protects updates to bdi_list.
  */
 DEFINE_SPINLOCK(bdi_lock);
 LIST_HEAD(bdi_list);
-LIST_HEAD(bdi_pending_list);
 
 static struct task_struct *sync_supers_tsk;
 static struct timer_list sync_supers_timer;
@@ -50,8 +47,6 @@ static struct timer_list sync_supers_tim
 static int bdi_sync_supers(void *);
 static void sync_supers_timer_fn(unsigned long);
 
-static void bdi_add_default_flusher_task(struct backing_dev_info *bdi);
-
 #ifdef CONFIG_DEBUG_FS
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
@@ -266,21 +261,8 @@ int bdi_has_dirty_io(struct backing_dev_
 	return wb_has_dirty_io(&bdi->wb);
 }
 
-static void bdi_flush_io(struct backing_dev_info *bdi)
-{
-	struct writeback_control wbc = {
-		.sync_mode		= WB_SYNC_NONE,
-		.older_than_this	= NULL,
-		.range_cyclic		= 1,
-		.nr_to_write		= 1024,
-	};
-
-	writeback_inodes_wb(&bdi->wb, &wbc);
-}
-
 /*
- * kupdated() used to do this. We cannot do it from the bdi_forker_task()
- * or we risk deadlocking on ->s_umount. The longer term solution would be
+ * kupdated() used to do this.  The longer term solution would be
  * to implement sync_supers_bdi() or similar and simply do it from the
  * bdi writeback tasks individually.
  */
@@ -318,160 +300,38 @@ static void sync_supers_timer_fn(unsigne
 	bdi_arm_supers_timer();
 }
 
-static int bdi_forker_task(void *ptr)
-{
-	struct bdi_writeback *me = ptr;
-
-	current->flags |= PF_FLUSHER | PF_SWAPWRITE;
-	set_freezable();
-
-	/*
-	 * Our parent may run at a different priority, just set us to normal
-	 */
-	set_user_nice(current, 0);
-
-	for (;;) {
-		struct backing_dev_info *bdi, *tmp;
-		struct bdi_writeback *wb;
-
-		/*
-		 * Temporary measure, we want to make sure we don't see
-		 * dirty data on the default backing_dev_info
-		 */
-		if (wb_has_dirty_io(me) || !list_empty(&me->bdi->work_list))
-			wb_do_writeback(me, 0);
-
-		spin_lock_bh(&bdi_lock);
-
-		/*
-		 * Check if any existing bdi's have dirty data without
-		 * a thread registered. If so, set that up.
-		 */
-		list_for_each_entry_safe(bdi, tmp, &bdi_list, bdi_list) {
-			if (bdi->wb.task)
-				continue;
-			if (list_empty(&bdi->work_list) &&
-			    !bdi_has_dirty_io(bdi))
-				continue;
-
-			bdi_add_default_flusher_task(bdi);
-		}
-
-		set_current_state(TASK_INTERRUPTIBLE);
-
-		if (list_empty(&bdi_pending_list)) {
-			unsigned long wait;
-
-			spin_unlock_bh(&bdi_lock);
-			wait = msecs_to_jiffies(dirty_writeback_interval * 10);
-			if (wait)
-				schedule_timeout(wait);
-			else
-				schedule();
-			try_to_freeze();
-			continue;
-		}
-
-		__set_current_state(TASK_RUNNING);
-
-		/*
-		 * This is our real job - check for pending entries in
-		 * bdi_pending_list, and create the tasks that got added
-		 */
-		bdi = list_entry(bdi_pending_list.next, struct backing_dev_info,
-				 bdi_list);
-		list_del_init(&bdi->bdi_list);
-		spin_unlock_bh(&bdi_lock);
-
-		wb = &bdi->wb;
-		wb->task = kthread_run(bdi_writeback_thread, wb, "flush-%s",
-					dev_name(bdi->dev));
-		/*
-		 * If task creation fails, then readd the bdi to
-		 * the pending list and force writeout of the bdi
-		 * from this forker thread. That will free some memory
-		 * and we can try again.
-		 */
-		if (IS_ERR(wb->task)) {
-			wb->task = NULL;
-
-			/*
-			 * Add this 'bdi' to the back, so we get
-			 * a chance to flush other bdi's to free
-			 * memory.
-			 */
-			spin_lock_bh(&bdi_lock);
-			list_add_tail(&bdi->bdi_list, &bdi_pending_list);
-			spin_unlock_bh(&bdi_lock);
-
-			bdi_flush_io(bdi);
-		}
-	}
-
-	return 0;
-}
-
-static void bdi_add_to_pending(struct rcu_head *head)
-{
-	struct backing_dev_info *bdi;
-
-	bdi = container_of(head, struct backing_dev_info, rcu_head);
-	INIT_LIST_HEAD(&bdi->bdi_list);
-
-	spin_lock(&bdi_lock);
-	list_add_tail(&bdi->bdi_list, &bdi_pending_list);
-	spin_unlock(&bdi_lock);
-
-	/*
-	 * We are now on the pending list, wake up bdi_forker_task()
-	 * to finish the job and add us back to the active bdi_list
-	 */
-	wake_up_process(default_backing_dev_info.wb.task);
-}
-
-/*
- * Add the default flusher task that gets created for any bdi
- * that has dirty data pending writeout
- */
-void static bdi_add_default_flusher_task(struct backing_dev_info *bdi)
+static int bdi_writeback_enable(struct backing_dev_info *bdi,
+		struct device *dev)
 {
-	if (!bdi_cap_writeback_dirty(bdi))
-		return;
-
-	if (WARN_ON(!test_bit(BDI_registered, &bdi->state))) {
-		printk(KERN_ERR "bdi %p/%s is not registered!\n",
-							bdi, bdi->name);
-		return;
+	bdi->wb.task = kthread_run(bdi_writeback_thread, &bdi->wb,
+				   "bdi-%s", dev_name(dev));
+	if (IS_ERR(bdi->wb.task)) {
+		bdi->wb.task = NULL;
+		return -ENOMEM;
 	}
 
-	/*
-	 * Check with the helper whether to proceed adding a task. Will only
-	 * abort if we two or more simultanous calls to
-	 * bdi_add_default_flusher_task() occured, further additions will block
-	 * waiting for previous additions to finish.
-	 */
-	if (!test_and_set_bit(BDI_pending, &bdi->state)) {
-		list_del_rcu(&bdi->bdi_list);
+	spin_lock_bh(&bdi_lock);
+	list_add_rcu(&bdi->bdi_list, &bdi_list);
+	spin_unlock_bh(&bdi_lock);
 
-		/*
-		 * We must wait for the current RCU period to end before
-		 * moving to the pending list. So schedule that operation
-		 * from an RCU callback.
-		 */
-		call_rcu(&bdi->rcu_head, bdi_add_to_pending);
-	}
+	return 0;
 }
 
-/*
- * Remove bdi from bdi_list, and ensure that it is no longer visible
- */
-static void bdi_remove_from_list(struct backing_dev_info *bdi)
+static void bdi_writeback_disable(struct backing_dev_info *bdi)
 {
 	spin_lock_bh(&bdi_lock);
 	list_del_rcu(&bdi->bdi_list);
 	spin_unlock_bh(&bdi_lock);
 
 	synchronize_rcu();
+
+	/*
+	 * Force unfreeze of the thread before calling kthread_stop(),
+	 * otherwise it would never exit if it is currently stuck in the
+	 * refrigerator.
+	 */
+	thaw_process(bdi->wb.task);
+	kthread_stop(bdi->wb.task);
 }
 
 int bdi_register(struct backing_dev_info *bdi, struct device *parent,
@@ -492,10 +352,6 @@ int bdi_register(struct backing_dev_info
 		goto exit;
 	}
 
-	spin_lock_bh(&bdi_lock);
-	list_add_tail_rcu(&bdi->bdi_list, &bdi_list);
-	spin_unlock_bh(&bdi_lock);
-
 	bdi->dev = dev;
 
 	/*
@@ -503,18 +359,10 @@ int bdi_register(struct backing_dev_info
 	 * and add other bdi's to the list. They will get a thread created
 	 * on-demand when they need it.
 	 */
-	if (bdi_cap_flush_forker(bdi)) {
-		struct bdi_writeback *wb = &bdi->wb;
-
-		wb->task = kthread_run(bdi_forker_task, wb, "bdi-%s",
-						dev_name(dev));
-		if (IS_ERR(wb->task)) {
-			wb->task = NULL;
-			ret = -ENOMEM;
-
-			bdi_remove_from_list(bdi);
+	if (bdi_cap_writeback_dirty(bdi)) {
+		ret = bdi_writeback_enable(bdi, dev);
+		if (ret)
 			goto exit;
-		}
 	}
 
 	bdi_debug_register(bdi, dev_name(dev));
@@ -532,37 +380,6 @@ int bdi_register_dev(struct backing_dev_
 EXPORT_SYMBOL(bdi_register_dev);
 
 /*
- * Remove bdi from the global list and shutdown any threads we have running
- */
-static void bdi_wb_shutdown(struct backing_dev_info *bdi)
-{
-	if (!bdi_cap_writeback_dirty(bdi))
-		return;
-
-	/*
-	 * If setup is pending, wait for that to complete first
-	 */
-	wait_on_bit(&bdi->state, BDI_pending, bdi_sched_wait,
-			TASK_UNINTERRUPTIBLE);
-
-	/*
-	 * Make sure nobody finds us on the bdi_list anymore
-	 */
-	bdi_remove_from_list(bdi);
-
-	/*
-	 * Finally, kill the kernel thread. We don't need to be RCU
-	 * safe anymore, since the bdi is gone from visibility. Force
-	 * unfreeze of the thread before calling kthread_stop(), otherwise
-	 * it would never exet if it is currently stuck in the refrigerator.
-	 */
-	if (bdi->wb.task) {
-		thaw_process(bdi->wb.task);
-		kthread_stop(bdi->wb.task);
-	}
-}
-
-/*
  * This bdi is going away now, make sure that no super_blocks point to it
  */
 static void bdi_prune_sb(struct backing_dev_info *bdi)
@@ -583,8 +400,8 @@ void bdi_unregister(struct backing_dev_i
 		trace_writeback_bdi_unregister(bdi);
 		bdi_prune_sb(bdi);
 
-		if (!bdi_cap_flush_forker(bdi))
-			bdi_wb_shutdown(bdi);
+		if (bdi_cap_writeback_dirty(bdi))
+			bdi_writeback_disable(bdi);
 		bdi_debug_unregister(bdi);
 		device_unregister(bdi->dev);
 		bdi->dev = NULL;
@@ -602,7 +419,6 @@ int bdi_init(struct backing_dev_info *bd
 	bdi->max_ratio = 100;
 	bdi->max_prop_frac = PROP_FRAC_BASE;
 	spin_lock_init(&bdi->wb_lock);
-	INIT_RCU_HEAD(&bdi->rcu_head);
 	INIT_LIST_HEAD(&bdi->bdi_list);
 	INIT_LIST_HEAD(&bdi->work_list);
 
@@ -631,20 +447,6 @@ void bdi_destroy(struct backing_dev_info
 {
 	int i;
 
-	/*
-	 * Splice our entries to the default_backing_dev_info, if this
-	 * bdi disappears
-	 */
-	if (bdi_has_dirty_io(bdi)) {
-		struct bdi_writeback *dst = &default_backing_dev_info.wb;
-
-		spin_lock(&inode_lock);
-		list_splice(&bdi->wb.b_dirty, &dst->b_dirty);
-		list_splice(&bdi->wb.b_io, &dst->b_io);
-		list_splice(&bdi->wb.b_more_io, &dst->b_more_io);
-		spin_unlock(&inode_lock);
-	}
-
 	bdi_unregister(bdi);
 
 	for (i = 0; i < NR_BDI_STAT_ITEMS; i++)
Index: linux-2.6/include/trace/events/writeback.h
===================================================================
--- linux-2.6.orig/include/trace/events/writeback.h	2010-07-07 11:33:56.285687937 -0700
+++ linux-2.6/include/trace/events/writeback.h	2010-07-07 11:35:35.586687939 -0700
@@ -45,7 +45,6 @@ DECLARE_EVENT_CLASS(writeback_work_class
 DEFINE_EVENT(writeback_work_class, name, \
 	TP_PROTO(struct backing_dev_info *bdi, struct wb_writeback_work *work), \
 	TP_ARGS(bdi, work))
-DEFINE_WRITEBACK_WORK_EVENT(writeback_nothread);
 DEFINE_WRITEBACK_WORK_EVENT(writeback_queue);
 DEFINE_WRITEBACK_WORK_EVENT(writeback_exec);
 
@@ -82,8 +81,6 @@ DEFINE_EVENT(writeback_class, name, \
 DEFINE_WRITEBACK_EVENT(writeback_nowork);
 DEFINE_WRITEBACK_EVENT(writeback_bdi_register);
 DEFINE_WRITEBACK_EVENT(writeback_bdi_unregister);
-DEFINE_WRITEBACK_EVENT(writeback_thread_start);
-DEFINE_WRITEBACK_EVENT(writeback_thread_stop);
 
 DECLARE_EVENT_CLASS(wbc_class,
 	TP_PROTO(struct writeback_control *wbc, struct backing_dev_info *bdi),
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux