lazytime fixes and cleanups

From: Eric Biggers <[email protected]>

When lazytime is enabled and an inode with dirty timestamps is being expired, either due to dirtytime_expire_interval being exceeded or due to a sync or syncfs system call, we need to inform the filesystem that the inode is dirty so that the inode's timestamps can be copied out to the on-disk data structures. That's because if the filesystem supports lazytime, it will have ignored the ->dirty_inode(inode, I_DIRTY_TIME) notification when the timestamp was modified in memory.

Currently this is accomplished by calling mark_inode_dirty_sync() from __writeback_single_inode(). However, this has the unfortunate side effect of also putting the inode the writeback list. That's not appropriate in this case, since the inode is already being written.

That causes the inode to remain dirty after a sync. Normally that's just wasteful, as it causes the inode to be written twice. But when fscrypt is used this bug also partially breaks the FS_IOC_REMOVE_ENCRYPTION_KEY ioctl, as the ioctl reports that files are still in-use when they aren't. For more details, see the original report at https://lore.kernel.org/r/[email protected]

Fix this by calling ->dirty_inode(inode, I_DIRTY_SYNC) directly instead of mark_inode_dirty_sync().

This fixes xfstest generic/580 when lazytime is enabled.

A later patch will introduce a ->lazytime_expired method to cleanly separate out the lazytime expiration case, in particular for XFS which uses the VFS-level dirtiness tracking only for lazytime. But that's separate from fixing this bug. Also, note that XFS will incorrectly ignore the I_DIRTY_SYNC notification from __writeback_single_inode() both before and after this patch, as I_DIRTY_TIME was already cleared in i_state. Later patches will fix this separate bug.

Fixes 0ae45f63d4ef ("vfs: add support for a lazytime mount option")

Cc [email protected]

Signed-off-by Eric Biggers <[email protected]>

fs/fs-writeback.c +7 -3

index acfb55834af23..081e335cdee47 100644
@@ -1509,11 +1509,22 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
 
 	spin_unlock(&inode->i_lock);
 
-	if (dirty & I_DIRTY_TIME)
-		mark_inode_dirty_sync(inode);
 	/* Don't write the inode if only I_DIRTY_PAGES was set */
 	if (dirty & ~I_DIRTY_PAGES) {
-		int err = write_inode(inode, wbc);
+		int err;
+
+		/*
+		 * If the inode is being written due to a lazytime timestamp
+		 * expiration, then the filesystem needs to be notified about it
+		 * so that e.g. the filesystem can update on-disk fields and
+		 * journal the timestamp update.  Just calling write_inode()

I think attached patch (compile-tested only) should actually fix the problem as well without this special ->dirty_inode() call. It basically only moves the mark_inode_dirty_sync() before inode->i_state clearing. Because conceptually mark_inode_dirty_sync() is IMO the right function to call. It will take care of clearing I_DIRTY_TIME flag (because we are setting I_DIRTY_SYNC), it will also not touch inode->i_io_list if the inode is queued for sync (I_SYNC_QUEUED is set in that case). The only problem with calling it was that it was called *after* clearing dirty bits from i_state... What do you think?

Quoted 20 lines

fs/fs-writeback.c +7 -3

index acfb55834af23..081e335cdee47 100644
@@ -1509,11 +1509,22 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
 
 	spin_unlock(&inode->i_lock);
 
-	if (dirty & I_DIRTY_TIME)
-		mark_inode_dirty_sync(inode);
 	/* Don't write the inode if only I_DIRTY_PAGES was set */
 	if (dirty & ~I_DIRTY_PAGES) {
-		int err = write_inode(inode, wbc);
+		int err;
+
+		/*
+		 * If the inode is being written due to a lazytime timestamp
+		 * expiration, then the filesystem needs to be notified about it
+		 * so that e.g. the filesystem can update on-disk fields and
+		 * journal the timestamp update.  Just calling write_inode()

fs/fs-writeback.c __writeback_single_inode(struct inode *inode, struct writeback_control *wbc) · line 1530

Quoted 10 lines

fs/fs-writeback.c +7 -0

+		 * isn't enough.  Don't call mark_inode_dirty_sync(), as that
+		 * would put the inode back on the dirty list.
+		 */
+		if ((dirty & I_DIRTY_TIME) && inode->i_sb->s_op->dirty_inode)
+			inode->i_sb->s_op->dirty_inode(inode, I_DIRTY_SYNC);
+
+		err = write_inode(inode, wbc);
 		if (ret == 0)
 			ret = err;
 	}

I have to say I dislike this special call of ->dirty_inode(). It works but it makes me wonder, didn't we forget about something or won't we forget in the future? Because it's very easy to miss this special case...

I think attached patch (compile-tested only) should actually fix the problem as well without this special ->dirty_inode() call. It basically only moves the mark_inode_dirty_sync() before inode->i_state clearing. Because conceptually mark_inode_dirty_sync() is IMO the right function to call. It will take care of clearing I_DIRTY_TIME flag (because we are setting I_DIRTY_SYNC), it will also not touch inode->i_io_list if the inode is queued for sync (I_SYNC_QUEUED is set in that case). The only problem with calling it was that it was called *after* clearing dirty bits from i_state... What do you think?

Honza

Signature · 12 lines

Jan Kara <[email protected]>
SUSE Labs, CR

From 80ccc6a78d1c0532f600b98884f7a64e58333485 Mon Sep 17 00:00:00 2001
From: Jan Kara <[email protected]>
Date: Thu, 7 Jan 2021 15:36:05 +0100
Subject: [PATCH] fs: Make sure inode is clean after __writeback_single_inode()

Signed-off-by: Jan Kara <[email protected]>
---
 fs/fs-writeback.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

fs/fs-writeback.c +12 -9

index acfb55834af2..b9356f470fae 100644
@@ -1473,22 +1473,25 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
 			ret = err;
 	}
 
+	/*
+	 * If inode has dirty timestamps and we need to write them, call
+	 * mark_inode_dirty_sync() to notify filesystem about it.
+	 */
+	if (inode->i_state & I_DIRTY_TIME &&
+	    (wbc->for_sync || wbc->sync_mode == WB_SYNC_ALL ||
+	     time_after(jiffies, inode->dirtied_time_when +
+			dirtytime_expire_interval * HZ))) {
+		trace_writeback_lazytime(inode);
+		mark_inode_dirty_sync(inode);
+	}
+
 	/*
 	 * Some filesystems may redirty the inode during the writeback
 	 * due to delalloc, clear dirty metadata flags right before
 	 * write_inode()
 	 */
 	spin_lock(&inode->i_lock);
-
 	dirty = inode->i_state & I_DIRTY;
-	if ((inode->i_state & I_DIRTY_TIME) &&
-	    ((dirty & I_DIRTY_INODE) ||
-	     wbc->sync_mode == WB_SYNC_ALL || wbc->for_sync ||
-	     time_after(jiffies, inode->dirtied_time_when +
-			dirtytime_expire_interval * HZ))) {
-		dirty |= I_DIRTY_TIME;
-		trace_writeback_lazytime(inode);
-	}
 	inode->i_state &= ~dirty;
 
 	/*

It looks like that's going to work, and it fixes the XFS bug too.

Note that if __writeback_single_inode() is called from writeback_single_inode() (rather than writeback_sb_inodes()), then the inode might not be queued for sync, in which case mark_inode_dirty_sync() will move it to a writeback list.

That's okay because afterwards, writeback_single_inode() will delete the inode from any writeback list if it's been fully cleaned, right? So clean inodes won't get left on a writeback list.

It's confusing because there are comments in writeback_single_inode() and above __writeback_single_inode() that say that the inode must not be moved between writeback lists. I take it that those comments are outdated, as they predate I_SYNC_QUEUED being introduced by commit 5afced3bf281 ("writeback: Avoid skipping inode writeback")?

fs/fs-writeback.c __writeback_single_inode(struct inode *inode, struct writeback_control *wbc) · line 1483

Quoted 8 lines

fs/fs-writeback.c +8 -0

+	/*
+	 * If inode has dirty timestamps and we need to write them, call
+	 * mark_inode_dirty_sync() to notify filesystem about it.
+	 */
+	if (inode->i_state & I_DIRTY_TIME &&
+	    (wbc->for_sync || wbc->sync_mode == WB_SYNC_ALL ||
+	     time_after(jiffies, inode->dirtied_time_when +
+			dirtytime_expire_interval * HZ))) {

If we're touching this area, it would be nice to split this condition into a readable helper ala:

static inline bool inode_needs_timestamp_sync(struct writeback_control *wbc,
struct inode *inode)

{
	if (!(inode->i_state & I_DIRTY_TIME))
		return false;
	if (wbc->for_sync || wbc->sync_mode == WB_SYNC_ALL)
		return true;
	return time_after(jiffies, inode->dirtied_time_when +
			  dirtytime_expire_interval * HZ);
}

From: Eric Biggers <[email protected]>

As was done for generic_update_time(), only pass I_DIRTY_TIME to __mark_inode_dirty() when the inode's timestamps were actually updated and lazytime is enabled. This avoids a weird edge case where I_DIRTY_TIME could be set in i_state when lazytime isn't enabled.

Signed-off-by Eric Biggers <[email protected]>

Quoted 8 lines · Eric Biggers

From: Eric Biggers <[email protected]>

As was done for generic_update_time(), only pass I_DIRTY_TIME to __mark_inode_dirty() when the inode's timestamps were actually updated and lazytime is enabled. This avoids a weird edge case where I_DIRTY_TIME could be set in i_state when lazytime isn't enabled.

Signed-off-by Eric Biggers <[email protected]>

...

Quoted 3 lines

fs/fat/misc.c +2 -0

+	if ((flags & S_VERSION) && inode_maybe_inc_iversion(inode, false))
+		dirty_flags |= I_DIRTY_SYNC;

fs/fat/misc.c EXPORT_SYMBOL_GPL(fat_truncate_time); · line 348

Quoted 3 lines

fs/fat/misc.c +0 -2

-	if (dirty)
-		iflags |= I_DIRTY_SYNC;
 	__mark_inode_dirty(inode, iflags);

^^^ dirty_flags here?

Otherwise the change looks good to me.

From: Eric Biggers <[email protected]>

There is no need to call ->dirty_inode for lazytime timestamp updates (i.e. for __mark_inode_dirty(I_DIRTY_TIME)), since by the definition of lazytime, filesystems must ignore these updates. Filesystems only need to care about the updated timestamps when they expire.

Therefore, only call ->dirty_inode when I_DIRTY_INODE is set.

Based on a patch from Christoph Hellwig:
https://lore.kernel.org/r/[email protected]

Signed-off-by Eric Biggers <[email protected]>

fs/fs-writeback.c +0 -0

index 081e335cdee47..e3347fd6eb13a 100644

Quoted 13 lines · Eric Biggers

From: Eric Biggers <[email protected]>

There is no need to call ->dirty_inode for lazytime timestamp updates (i.e. for __mark_inode_dirty(I_DIRTY_TIME)), since by the definition of lazytime, filesystems must ignore these updates. Filesystems only need to care about the updated timestamps when they expire.

Therefore, only call ->dirty_inode when I_DIRTY_INODE is set.

Based on a patch from Christoph Hellwig:
https://lore.kernel.org/r/[email protected]

Signed-off-by Eric Biggers <[email protected]>

...

Quoted 12 lines

fs/fs-writeback.c +1 -1

index 081e335cdee47..e3347fd6eb13a 100644
@@ -2264,16 +2264,16 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 	 * Don't do this for I_DIRTY_PAGES - that doesn't actually
 	 * dirty the inode itself
 	 */
-	if (flags & (I_DIRTY_INODE | I_DIRTY_TIME)) {
+	if (flags & I_DIRTY_INODE) {
 		trace_writeback_dirty_inode_start(inode, flags);
 

fs/fs-writeback.c void __mark_inode_dirty(struct inode *inode, int flags) · line 2271

Quoted 2 lines

fs/fs-writeback.c +0 -0

 		if (sb->s_op->dirty_inode)
 			sb->s_op->dirty_inode(inode, flags);

OK, but shouldn't we pass just (flags & I_DIRTY_INODE) to ->dirty_inode(). Just to make it clear what the filesystem is supposed to consume in 'flags'...

From: Eric Biggers <[email protected]>

Since I_DIRTY_TIME and I_DIRTY_INODE are mutually exclusive in i_state, there's no need to check for I_DIRTY_TIME && !I_DIRTY_INODE. Just check for I_DIRTY_TIME.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/ext4/inode.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

fs/ext4/inode.c +0 -0

index 4cc6c7834312f..9e34541715968 100644

Quoted 16 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Since I_DIRTY_TIME and I_DIRTY_INODE are mutually exclusive in i_state, there's no need to check for I_DIRTY_TIME && !I_DIRTY_INODE. Just check for I_DIRTY_TIME.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/ext4/inode.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

fs/ext4/inode.c +0 -0

index 4cc6c7834312f..9e34541715968 100644

fs/ext4/inode.c static void __ext4_update_other_inode_time(struct super_block *sb, · line 4966

Quoted 7 lines

fs/ext4/inode.c +1 -2

@@ -4962,14 +4962,12 @@ static void __ext4_update_other_inode_time(struct super_block *sb,
 		return;
 
 	if ((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
-			       I_DIRTY_INODE)) ||
-	    ((inode->i_state & I_DIRTY_TIME) == 0))
+			       I_DIRTY_TIME)) != I_DIRTY_TIME)
 		return;

This is OK.

fs/ext4/inode.c static void __ext4_update_other_inode_time(struct super_block *sb, · line 4970

Quoted 6 lines

fs/ext4/inode.c +2 -3

 	spin_lock(&inode->i_lock);
-	if (((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
-				I_DIRTY_INODE)) == 0) &&
-	    (inode->i_state & I_DIRTY_TIME)) {
+	if ((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
+			       I_DIRTY_TIME)) != I_DIRTY_TIME) {

But this condition is negated AFAICT. We should have == I_DIRTY_TIME here AFAICT.

From: Eric Biggers <[email protected]>

Add a lazytime_expired method to 'struct super_operations'. Filesystems can implement this to be notified when an inode's lazytime timestamps have expired and need to be written to disk.

This avoids any potential ambiguity with ->dirty_inode(inode, I_DIRTY_SYNC), which can also mean a generic dirtying of the inode, not just a lazytime timestamp expiration. In particular, this will be useful for XFS.

If not implemented, then ->dirty_inode(inode, I_DIRTY_SYNC) continues to be called.

Note that there are three cases where we have to make sure to call lazytime_expired():

- __writeback_single_inode(): inode is being written now
- vfs_fsync_range(): inode is going to be synced
- iput(): inode is going to be evicted

In the latter two cases, the inode still needs to be put on the writeback list. So, we can't just replace the calls to mark_inode_dirty_sync() with lazytime_expired(). Instead, add a new flag I_DIRTY_TIME_EXPIRED which can be passed to __mark_inode_dirty(). It's like I_DIRTY_SYNC, except it causes the filesystem to be notified of a lazytime expiration rather than a generic I_DIRTY_SYNC.

Signed-off-by Eric Biggers <[email protected]>

---
 Documentation/filesystems/locking.rst |  2 ++
 Documentation/filesystems/vfs.rst     | 10 ++++++++++
 fs/fs-writeback.c                     | 27 ++++++++++++++++++++++-----
 fs/inode.c                            |  2 +-
 fs/sync.c                             |  2 +-
 include/linux/fs.h                    |  7 ++++++-
 6 files changed, 42 insertions(+), 8 deletions(-)

Documentation/filesystems/locking.rst +2 -0

index c0f2c7586531b..53088e2a93b69 100644
@@ -150,6 +150,7 @@ prototypes::
 	void (*free_inode)(struct inode *);
 	void (*destroy_inode)(struct inode *);
 	void (*dirty_inode) (struct inode *, int flags);
+	void (*lazytime_expired) (struct inode *);
 	int (*write_inode) (struct inode *, struct writeback_control *wbc);
 	int (*drop_inode) (struct inode *);
 	void (*evict_inode) (struct inode *);

@@ -175,6 +176,7 @@ alloc_inode:
 free_inode:				called from RCU callback
 destroy_inode:
 dirty_inode:
+lazytime_expired:
 write_inode:
 drop_inode:				!!!inode->i_lock!!!
 evict_inode:

Documentation/filesystems/vfs.rst +10 -0

index 287b80948a40b..02531b1421d01 100644
@@ -231,6 +231,7 @@ filesystem.  As of kernel 2.6.22, the following members are defined:
 		void (*destroy_inode)(struct inode *);
 
 		void (*dirty_inode) (struct inode *, int flags);
+		void (*lazytime_expired) (struct inode *);
 		int (*write_inode) (struct inode *, int);
 		void (*drop_inode) (struct inode *);
 		void (*delete_inode) (struct inode *);

@@ -275,6 +276,15 @@ or bottom half).
 	not its data.  If the update needs to be persisted by fdatasync(),
 	then I_DIRTY_DATASYNC will be set in the flags argument.
 
+``lazytime_expired``
+	when the lazytime mount option is enabled, this method is
+	called when an inode's in-memory updated timestamps have
+	expired and thus need to be written to disk.  This happens
+	when the timestamps have been in memory for too long, when the
+	inode is going to be evicted, or when userspace triggers a
+	sync.  If this method is not implemented, then
+	->dirty_inode(inode, I_DIRTY_SYNC) is called instead.
+
 ``write_inode``
 	this method is called when the VFS needs to write an inode to
 	disc.  The second parameter indicates whether the write should

fs/fs-writeback.c +22 -5

index ed76112bd067b..f187fc3f854e4 100644
@@ -1441,6 +1441,14 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
 	}
 }
 
+static void lazytime_expired(struct inode *inode)
+{
+	if (inode->i_sb->s_op->lazytime_expired)
+		inode->i_sb->s_op->lazytime_expired(inode);
+	else if (inode->i_sb->s_op->dirty_inode)
+		inode->i_sb->s_op->dirty_inode(inode, I_DIRTY_SYNC);
+}
+
 /*
  * Write out an inode and its dirty pages. Do not update the writeback list
  * linkage. That is left to the caller. The caller is also responsible for

@@ -1520,8 +1528,8 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
 		 * isn't enough.  Don't call mark_inode_dirty_sync(), as that
 		 * would put the inode back on the dirty list.
 		 */
-		if ((dirty & I_DIRTY_TIME) && inode->i_sb->s_op->dirty_inode)
-			inode->i_sb->s_op->dirty_inode(inode, I_DIRTY_SYNC);
+		if (dirty & I_DIRTY_TIME)
+			lazytime_expired(inode);
 
 		err = write_inode(inode, wbc);
 		if (ret == 0)

@@ -2231,8 +2239,9 @@ static noinline void block_dump___mark_inode_dirty(struct inode *inode)
  *
  * @inode: inode to mark
  * @flags: what kind of dirty, e.g. I_DIRTY_SYNC.  This can be a combination of
- *	   multiple I_DIRTY_* flags, except that I_DIRTY_TIME can't be combined
- *	   with I_DIRTY_PAGES.
+ *	   multiple I_DIRTY_* flags, except that:
+ *	   - I_DIRTY_TIME can't be combined with I_DIRTY_PAGES
+ *	   - I_DIRTY_TIME_EXPIRED must be used by itself
  *
  * Mark an inode as dirty.  We notify the filesystem, then update the inode's
  * dirty flags.  Then, if needed we add the inode to the appropriate dirty list.

@@ -2260,7 +2269,15 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 
 	trace_writeback_mark_inode_dirty(inode, flags);
 
-	if (flags & I_DIRTY_INODE) {
+	if (flags & I_DIRTY_TIME_EXPIRED) {
+		/*
+		 * Notify the filesystem about a lazytime timestamp expiration.
+		 * Afterwards, this case just turns into I_DIRTY_SYNC.
+		 */
+		WARN_ON_ONCE(flags & ~I_DIRTY_TIME_EXPIRED);
+		lazytime_expired(inode);
+		flags = I_DIRTY_SYNC;
+	} else if (flags & I_DIRTY_INODE) {
 		/*
 		 * Notify the filesystem about the inode being dirtied, so that
 		 * (if needed) it can update on-disk fields and journal the

fs/inode.c +1 -1

index d0fa43d8e9d5c..039b201a4743a 100644
@@ -1673,7 +1673,7 @@ void iput(struct inode *inode)
 			atomic_inc(&inode->i_count);
 			spin_unlock(&inode->i_lock);
 			trace_writeback_lazytime_iput(inode);
-			mark_inode_dirty_sync(inode);
+			__mark_inode_dirty(inode, I_DIRTY_TIME_EXPIRED);
 			goto retry;
 		}
 		iput_final(inode);

fs/sync.c +1 -1

index 1373a610dc784..363071a3528e3 100644
@@ -196,7 +196,7 @@ int vfs_fsync_range(struct file *file, loff_t start, loff_t end, int datasync)
 	if (!file->f_op->fsync)
 		return -EINVAL;
 	if (!datasync && (inode->i_state & I_DIRTY_TIME))
-		mark_inode_dirty_sync(inode);
+		__mark_inode_dirty(inode, I_DIRTY_TIME_EXPIRED);
 	return file->f_op->fsync(file, start, end, datasync);
 }
 EXPORT_SYMBOL(vfs_fsync_range);

include/linux/fs.h +6 -1

index 45a0303b2aeb6..8c5f5c5e62be4 100644
@@ -1935,7 +1935,8 @@ struct super_operations {
 	void (*destroy_inode)(struct inode *);
 	void (*free_inode)(struct inode *);
 
-   	void (*dirty_inode) (struct inode *, int flags);
+	void (*dirty_inode) (struct inode *, int flags);
+	void (*lazytime_expired)(struct inode *);
 	int (*write_inode) (struct inode *, struct writeback_control *wbc);
 	int (*drop_inode) (struct inode *);
 	void (*evict_inode) (struct inode *);

@@ -2108,6 +2109,9 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
  *			(I_DIRTY_SYNC and/or I_DIRTY_DATASYNC) gets set.  I.e.
  *			either I_DIRTY_TIME *or* I_DIRTY_INODE can be set in
  *			i_state, but not both.  I_DIRTY_PAGES may still be set.
+ * I_DIRTY_TIME_EXPIRED Passed to __mark_inode_dirty() to indicate the intent to
+ *			expire the inode's timestamps.  Not stored in i_state.
+ *
  * I_NEW		Serves as both a mutex and completion notification.
  *			New inodes set I_NEW.  If two processes both create
  *			the same inode, one of them will release its inode and

@@ -2173,6 +2177,7 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
 #define I_DIO_WAKEUP		(1 << __I_DIO_WAKEUP)
 #define I_LINKABLE		(1 << 10)
 #define I_DIRTY_TIME		(1 << 11)
+#define I_DIRTY_TIME_EXPIRED	(1 << 12)
 #define I_WB_SWITCH		(1 << 13)
 #define I_OVL_INUSE		(1 << 14)
 #define I_CREATING		(1 << 15)

Signature · 1 line

2.30.0

Quoted 28 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Add a lazytime_expired method to 'struct super_operations'. Filesystems can implement this to be notified when an inode's lazytime timestamps have expired and need to be written to disk.

This avoids any potential ambiguity with ->dirty_inode(inode, I_DIRTY_SYNC), which can also mean a generic dirtying of the inode, not just a lazytime timestamp expiration. In particular, this will be useful for XFS.

If not implemented, then ->dirty_inode(inode, I_DIRTY_SYNC) continues to be called.

Note that there are three cases where we have to make sure to call lazytime_expired():

- __writeback_single_inode(): inode is being written now
- vfs_fsync_range(): inode is going to be synced
- iput(): inode is going to be evicted

In the latter two cases, the inode still needs to be put on the writeback list. So, we can't just replace the calls to mark_inode_dirty_sync() with lazytime_expired(). Instead, add a new flag I_DIRTY_TIME_EXPIRED which can be passed to __mark_inode_dirty(). It's like I_DIRTY_SYNC, except it causes the filesystem to be notified of a lazytime expiration rather than a generic I_DIRTY_SYNC.

Quoted 1 line

Signed-off-by Eric Biggers <[email protected]>

Hum, seeing this patch I kind of wonder: Why don't we dirty the inode after expiring the lazytime timestamps with I_DIRTY_SYNC | I_DIRTY_TIME_EXPIRED and propagate I_DIRTY_TIME_EXPIRED even to ->dirty_inode() where XFS can catch it and act? Functionally it would be the same but we'd save a bunch of generic code and ->lazytime_expired helper used just by a single filesystem...

The XFS developers might have a different opinion though, as they were the ones who requested it originally:

	https://lore.kernel.org/r/[email protected]
	https://lore.kernel.org/r/[email protected]
	https://lore.kernel.org/r/20200325154759.GY29339@magnolia
	https://lore.kernel.org/r/[email protected]

Any thoughts from anyone about whether we should still introduce a separate notification for lazytime expiration, vs. just using ->dirty_inode(I_DIRTY_SYNC) with I_DIRTY_TIME in i_state?

From: Eric Biggers <[email protected]>

The comment in xfs_file_aio_write_checks() about calling file_modified() after dropping the ilock doesn't make sense, because the code that unconditionally acquires and drops the ilock was removed by commit 467f78992a07 ("xfs: reduce ilock hold times in xfs_file_aio_write_checks").

Remove this outdated comment.