Re: [PATCH 1/3] writeback: Avoid skipping inode writeback

[Christoph Hellwig <hch@infradead.org>]

This generall looks ok, but a few nitpicks below:

> -static void redirty_tail(struct inode *inode, struct bdi_writeback *wb)
> +static void __redirty_tail(struct inode *inode, struct bdi_writeback *wb)

I think redirty_tail_locked would be a more decriptive name, and also
fit other uses in this file (e.g. inode_io_list_move_locked and
inode_io_list_del_locked).

>  {
> +	assert_spin_locked(&inode->i_lock);
>  	if (!list_empty(&wb->b_dirty)) {

Nit: I find an empty line after asserts and before the real code starts
nice on the eye.

>  			break;
>  		list_move(&inode->i_io_list, &tmp);
>  		moved++;
> +		spin_lock(&inode->i_lock);
>  		if (flags & EXPIRE_DIRTY_ATIME)
> -			set_bit(__I_DIRTY_TIME_EXPIRED, &inode->i_state);
> +			inode->i_state |= I_DIRTY_TIME_EXPIRED;
> +		inode->i_state |= I_SYNC_QUEUED;
> +		spin_unlock(&inode->i_lock);

I wonder if the locking changes should go into a prep patch vs the
actual logic changes related to I_SYNC_QUEUED?  That would untangle
the patch quite a bit and make it easier to follow.

>  #define I_WB_SWITCH		(1 << 13)
>  #define I_OVL_INUSE		(1 << 14)
>  #define I_CREATING		(1 << 15)
> +#define I_SYNC_QUEUED		(1 << 16)

FYI, this conflicts with the I_DONTCAT addition in mainline now.