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