On Tue, Mar 20, 2012 at 11:56:30PM +0100, Jan Kara wrote:
> The code in writeback_single_inode() is relatively complex. The list requeing
> logic makes sense only for flusher thread but not really for sync_inode() or
> write_inode_now() callers. Also when we want to get rid of inode references
> held by flusher thread, we will need a special I_SYNC handling there.
>
> So separate part of writeback_single_inode() which does the real writeback work
> into __writeback_single_inode() and make writeback_single_inode() do only stuff
> necessary for callers writing only one inode, moving the special list handling
> into writeback_sb_inodes(). As a sideeffect this fixes a possible race where we
> could skip some inode during sync(2) because other writer refiled it from b_io
> to b_dirty list. Also I_SYNC handling is moved into the callers of
> __writeback_single_inode() to make locking easier.
.....
> +static int
> +writeback_single_inode(struct inode *inode, struct bdi_writeback *wb,
> + struct writeback_control *wbc)
> +{
> + int ret = 0;
> +
> + spin_lock(&inode->i_lock);
> + if (!atomic_read(&inode->i_count))
> + WARN_ON(!(inode->i_state & (I_WILL_FREE|I_FREEING)));
> + else
> + WARN_ON(inode->i_state & I_WILL_FREE);
> +
> + if (inode->i_state & I_SYNC) {
> + if (wbc->sync_mode != WB_SYNC_ALL)
> + goto out;
> + /*
> + * It's a data-integrity sync. We must wait.
> + */
> + inode_wait_for_writeback(inode);
> + }
> + BUG_ON(inode->i_state & I_SYNC);
BUG_ON() seems a little harsh to me. I mean, having I_SYNC set is
not really a fatal error. It's an indication of a problem, but
writeback will continue and not fail catastrophically if this
occurs. So perhaps WARN_ON() might be better here.
.....
> @@ -576,23 +612,24 @@ static long writeback_sb_inodes(struct super_block *sb,
> spin_unlock(&wb->list_lock);
>
> __iget(inode);
> + if (inode->i_state & I_SYNC)
> + inode_wait_for_writeback(inode);
Even for WB_SYNC_NONE writeback?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
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
