On Wed 10-06-20 08:02:03, Christoph Hellwig wrote:
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).
Fair enough, will do.
{
- 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.
Sure.
break; list_move(&inode->i_io_list, &tmp); moved++;
spin_lock(&inode->i_lock);
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.
OK, will do.
#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.
Yup, I've already found out when rebasing...
Thanks for review!
Honza