Thread (33 messages) 33 messages, 7 authors, 2012-02-27

Re: reclaim the LRU lists full of dirty/writeback pages

From: Wu Fengguang <hidden>
Date: 2012-02-16 14:16:33

On Thu, Feb 16, 2012 at 09:32:33PM +0800, Wu Fengguang wrote:
On Thu, Feb 16, 2012 at 01:44:45PM +0100, Jan Kara wrote:
quoted
On Thu 16-02-12 12:00:19, Wu Fengguang wrote:
quoted
On Tue, Feb 14, 2012 at 02:29:50PM +0100, Jan Kara wrote:
quoted
quoted
quoted
quoted
quoted
quoted
+/*
+ * schedule writeback on a range of inode pages.
+ */
+static struct wb_writeback_work *
+bdi_flush_inode_range(struct backing_dev_info *bdi,
+		      struct inode *inode,
+		      pgoff_t offset,
+		      pgoff_t len,
+		      bool wait)
+{
+	struct wb_writeback_work *work;
+
+	if (!igrab(inode))
+		return ERR_PTR(-ENOENT);
  One technical note here: If the inode is deleted while it is queued, this
reference will keep it living until flusher thread gets to it. Then when
flusher thread puts its reference, the inode will get deleted in flusher
thread context. I don't see an immediate problem in that but it might be
surprising sometimes. Another problem I see is that if you try to
unmount the filesystem while the work item is queued, you'll get EBUSY for
no apparent reason (for userspace).
Yeah, we need to make umount work.
  The positive thing is that if the inode is reaped while the work item is
queue, we know all that needed to be done is done. So we don't really need
to pin the inode.
But I do need to make sure the *inode pointer does not point to some
invalid memory at work exec time. Is this possible without raising
->i_count?
  I was thinking about it and what should work is that we have inode
reference in work item but in generic_shutdown_super() we go through
the worklist and drop all work items for superblock before calling
evict_inodes()...
Good point!

This diff removes the works after the sync_filesystem(sb) call.  After
which, no more dirty pages are expected on that sb (otherwise the
umount will fail anyway), hence no more pageout works will be queued
for that sb.

+static void wb_free_work(struct wb_writeback_work *work)
+{
+	/*
+	 * Notify the caller of completion if this is a synchronous
+	 * work item, otherwise just free it.
+	 */
+	if (work->done)
+		complete(work->done);
+	else
+		mempool_free(work, wb_work_mempool);
+}
+
+/*
+ * Remove works for @sb; or if (@sb == NULL), remove all works on @bdi.
+ */
+void bdi_remove_works(struct backing_dev_info *bdi, struct super_block *sb)
+{
+	struct inode *inode = mapping->host;
+	struct wb_writeback_work *work;
+
+	spin_lock_bh(&bdi->wb_lock);
+	list_for_each_entry_safe(work, &bdi->work_list, list) {
+		if (work->inode && work->inode->i_sb == sb) {
+			iput(inode);
+		} else if (sb && work->sb != sb)
+			continue;
+
+		list_del_init(&work->list);
+		wb_free_work(work);
+	}
+	spin_unlock_bh(&bdi->wb_lock);
+}
Sorry, this corrected function actually compiles:

+/* 
+ * Remove works for @sb; or if (@sb == NULL), remove all works on @bdi. 
+ */ 
+void bdi_remove_works(struct backing_dev_info *bdi, struct super_block *sb) 
+{ 
+       struct wb_writeback_work *work, *tmp; 
+       LIST_HEAD(works); 
+ 
+       spin_lock_bh(&bdi->wb_lock); 
+       list_for_each_entry_safe(work, tmp, &bdi->work_list, list) { 
+               if (sb) { 
+                       if (work->sb && work->sb != sb) 
+                               continue; 
+                       if (work->inode && work->inode->i_sb != sb) 
+                               continue; 
+               } 
+               list_move(&work->list, &works); 
+       } 
+       spin_unlock_bh(&bdi->wb_lock); 
+ 
+       while (!list_empty(&works)) { 
+               work = list_entry(works.next, 
+                                 struct wb_writeback_work, list); 
+               list_del_init(&work->list); 
+               if (work->inode) 
+                       iput(work->inode); 
+               wb_free_work(work); 
+       } 
+} 
quoted hunk ↗ jump to hunk
--- linux.orig/fs/super.c	2012-02-16 21:08:09.000000000 +0800
+++ linux/fs/super.c	2012-02-16 21:22:19.000000000 +0800
@@ -389,6 +389,7 @@ void generic_shutdown_super(struct super
 
 		fsnotify_unmount_inodes(&sb->s_inodes);
 
+		bdi_remove_works(sb->s_bdi, sb);
 		evict_inodes(sb);
 
 		if (sop->put_super)
Thanks,
Fengguang
--
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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help