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>