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@xxxxxxxxx. 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@xxxxxxxxx";> email@xxxxxxxxx </a>