On Tue, Nov 23, 2010 at 09:02:39PM +1100, Nick Piggin wrote:
> 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);
> + bdi_start_writeback(sb->s_bdi, get_nr_dirty_pages());
> return 1;
> - } else
> - return 0;
> + }
> + return 0;
> }
> EXPORT_SYMBOL(writeback_inodes_sb_if_idle);
>
> @@ -1172,17 +1173,18 @@ EXPORT_SYMBOL(writeback_inodes_sb_if_idl
> *
> * Invoke writeback_inodes_sb if no writeback is currently underway.
> * Returns 1 if writeback was started, 0 if not.
> + *
> + * Even if 1 is returned, writeback may not be started if memory allocation
> + * fails. This function makes no guarantees about anything.
> */
> int writeback_inodes_sb_nr_if_idle(struct super_block *sb,
> unsigned long nr)
> {
> if (!writeback_in_progress(sb->s_bdi)) {
> - down_read(&sb->s_umount);
> - writeback_inodes_sb_nr(sb, nr);
> - up_read(&sb->s_umount);
> + bdi_start_writeback(sb->s_bdi, nr);
> return 1;
> - } else
> - return 0;
> + }
> + return 0;
> }
So I changed btrfs's function too -- I don't think it's too sane to take
s_umount inside a function that advertises itself to be freely called by
filesystems and not having anything of its locking rules documented.
The issue of writeback_inodes_sb being synchronous so far as it has to
wait until the work has been dequeued is another subtlety. That is a
funny interface though, really. It has 3 callers, sync, quota, and
ubifs. From a quick look, quota and ubifs seem to think it is some kind
of synchronous writeout API. It also really sucks that it can get
deadlocked behind an unrelated item in a workqueue. I think it should
just get converted over to the async-submission style as well.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
