Re: [PATCH 2/2] fs: record I_DIRTY_TIME even if inode already has I_DIRTY_INODE

[Lukas Czerner <lczerner@redhat.com>]

On Thu, Jul 28, 2022 at 09:05:11PM -0700, Eric Biggers wrote:
> On Thu, Jul 28, 2022 at 03:39:14PM +0200, Lukas Czerner wrote:
> > Currently the I_DIRTY_TIME will never get set if the inode already has
> > I_DIRTY_INODE with assumption that it supersedes I_DIRTY_TIME.  That's
> > true, however ext4 will only update the on-disk inode in
> > ->dirty_inode(), not on actual writeback. As a result if the inode
> > already has I_DIRTY_INODE state by the time we get to
> > __mark_inode_dirty() only with I_DIRTY_TIME, the time was already filled
> > into on-disk inode and will not get updated until the next I_DIRTY_INODE
> > update, which might never come if we crash or get a power failure.
> > 
> > The problem can be reproduced on ext4 by running xfstest generic/622
> > with -o iversion mount option. Fix it by setting I_DIRTY_TIME even if
> > the inode already has I_DIRTY_INODE.
> > 
> > Also clear the I_DIRTY_TIME after ->dirty_inode() otherwise it may never
> > get cleared.
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> 
> If you're going to change the meaning of I_* flags, please update the comment in
> include/linux/fs.h that describes what they mean.
> 
> - Eric

Good point, it does say that I_DIRTY_TIME and I_DIRTY_INODE can't be
both set.

Thanks!
-Lukas