On Fri, Oct 29, 2010 at 02:23:33PM +1100, Dave Chinner wrote:
> @@ -293,9 +293,11 @@ static void inode_wait_for_writeback(struct inode *inode)
>
> wqh = bit_waitqueue(&inode->i_state, __I_SYNC);
> while (inode->i_state & I_SYNC) {
> + spin_unlock(&inode->i_lock);
Minor annoyance, but... Let's replace spaces with tab in that while.
> + spin_unlock(&inode->i_lock);
> +
> pages_skipped = wbc->pages_skipped;
> writeback_single_inode(inode, wbc);
Might make sense to lift locking i_lock into callers of
writeback_single_inode() (it has to grab the damn thing as soon as it's
called) and collapse it with spin_unlock() here. Separate patch.
> + spin_unlock(&inode->i_lock);
> /*
> * If the inode was already on b_dirty/b_io/b_more_io, don't
> * reposition it (that would break b_dirty time-ordering).
I'm not sure there's any point in dropping it here, actually.
> + spin_lock(&inode->i_lock);
> inode->i_state = I_FREEING | I_CLEAR;
> + spin_unlock(&inode->i_lock);
_Probably_ not needed here; we already had I_FREEING set by the time
we'd called that and no other thread will modify ->i_state of that
inode at that point. Check for I_CLEAR will be later in the same
thread, so no barriers are needed. Separate patch.
> @@ -552,8 +568,6 @@ int invalidate_inodes(struct super_block *sb)
> */
> list_move(&inode->i_lru, &dispose);
> list_del_init(&inode->i_wb_list);
> - if (!(inode->i_state & (I_DIRTY | I_SYNC)))
> - percpu_counter_dec(&nr_inodes_unused);
> }
> spin_unlock(&inode_lock);
Ho-hum...
I'm not sure we need that list_del_init() here, actually, seeing that
I_FREEING is already set... For later patch, anyway.
> @@ -917,10 +946,12 @@ static struct inode *get_new_inode_fast(struct super_block *sb,
> /* We released the lock, so.. */
> old = find_inode_fast(sb, head, ino);
> if (!old) {
> + spin_lock(&inode->i_lock);
> inode->i_ino = ino;
> + inode->i_state = I_NEW;
> + spin_unlock(&inode->i_lock);
Almost certainly not needed; nobody can find this inode at that point.
> + * wake_up_bit(&inode->i_state, __I_NEW) after removing from the hash list
> + * will DTRT.
Add that i_lock is not regained.
--
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
