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

[Wu Fengguang <fengguang.wu@intel.com>]

On Thu, Feb 16, 2012 at 01:44:45PM +0100, Jan Kara wrote:
> On Thu 16-02-12 12:00:19, Wu Fengguang wrote:
> > On Tue, Feb 14, 2012 at 02:29:50PM +0100, Jan Kara wrote:

> > > > > > +/*
> > > > > > + * 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);
+}

--- 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>