On Tue 08-06-10 18:14:51, Christoph Hellwig wrote:
> Make sure that not only sync_filesystem but all callers of writeback_inodes_sb
> have the superblock protected against remount. As-is this disables all
> functionality for these callers, but the next patch relies on this locking to
> fix writeback_inodes_sb for sync_filesystem.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> Index: linux-2.6/fs/fs-writeback.c
> ===================================================================
> --- linux-2.6.orig/fs/fs-writeback.c 2010-06-08 16:25:35.000000000 +0200
> +++ linux-2.6/fs/fs-writeback.c 2010-06-08 16:27:47.846255651 +0200
> @@ -1180,6 +1180,8 @@ void writeback_inodes_sb(struct super_bl
> .sync_mode = WB_SYNC_NONE,
> };
>
> + WARN_ON(!rwsem_is_locked(&sb->s_umount));
> +
> args.nr_pages = nr_dirty + nr_unstable +
> (inodes_stat.nr_inodes - inodes_stat.nr_unused);
>
> @@ -1197,7 +1199,9 @@ EXPORT_SYMBOL(writeback_inodes_sb);
> int writeback_inodes_sb_if_idle(struct super_block *sb)
> {
> if (!writeback_in_progress(sb->s_bdi)) {
> + down_read(&sb->s_umount);
> writeback_inodes_sb(sb);
> + up_read(&sb->s_umount);
But this is buggy isn't it? writeback_inodes_sb() returns as soon as
flusher threads start submitting the IO. So we release s_umount too early -
flusher threads can be still writing out inodes for our work. It's exactly
cases like this why I don't like the work to be called "done" when flusher
threads actually still do it...
Honza
> return 1;
> } else
> return 0;
> @@ -1220,6 +1224,8 @@ void sync_inodes_sb(struct super_block *
> .range_cyclic = 0,
> };
>
> + WARN_ON(!rwsem_is_locked(&sb->s_umount));
> +
> bdi_queue_work_onstack(&args);
> wait_sb_inodes(sb);
> }
> Index: linux-2.6/fs/ubifs/budget.c
> ===================================================================
> --- linux-2.6.orig/fs/ubifs/budget.c 2010-06-08 16:22:16.000000000 +0200
> +++ linux-2.6/fs/ubifs/budget.c 2010-06-08 16:26:50.464004778 +0200
> @@ -62,7 +62,9 @@
> */
> static void shrink_liability(struct ubifs_info *c, int nr_to_write)
> {
> + down_read(&c->vfs_sb->s_umount);
> writeback_inodes_sb(c->vfs_sb);
> + up_read(&c->vfs_sb->s_umount);
> }
>
> /**
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
