Re: [RESEND PATCH] writeback: don't requeue inode already on b_dirty

[Jan Kara]

 On Fri 23-10-26 10:37:07, Hongyu Jin wrote:
> Since commit 846a3351ddfe ("writeback: Avoid skipping inode writeback"),
> writeback_single_inode() calls redirty_tail_locked() for any inode left
> dirty. When that inode is already on b_dirty and older than the newest
> inode there, redirty_tail_locked() moves it to the head and stamps
> dirtied_when with jiffies.
> 
> Append to an f2fs file so the i_size update puts it on DIRTY_META, and
> dirty a newer inode before each checkpoint. Every 17s the checkpoint
> syncs that file and redirty_tail_locked() stamps a new dirtied_when.
> 17s is shorter than the default 30s dirty_expire_interval, so kupdate
> never expires it. Dirty pages pile up until writeback or fsync().
> 
> An inode that was I_DIRTY on entry is already on b_dirty, so only
> redirty it if it was not. The b_dirty_time case fixed by commit
> 846a3351ddfe ("writeback: Avoid skipping inode writeback") enters without
> I_DIRTY and is still handled.
> 
> Fixes: 846a3351ddfe ("writeback: Avoid skipping inode writeback")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hongyu Jin <hongyu.jin@xxxxxxxxxx>

I was mulling over this for quite some time (which is why I didn't reply to
your patch earlier - plus the fact that currently there a *lot* of things
going on). I agree the current behavior is problematic - in particular that
if sync_inode_metadata() is called frequently enough, we never get to
kupdate style data writeback.

But I wasn't quite sure your fix is correct. In the end I've convinced
myself it is fine but we can actually go even further and remove the
redirty_tail() logic altogether. So I'm now testing the attached patch.
If my testing passes, I'll send it for inclusion.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR