On Thu, Aug 28, 2025 at 10:26:50AM +1000, Dave Chinner wrote:

On Tue, Aug 26, 2025 at 11:39:53AM -0400, Josef Bacik wrote:

quoted

If the inode is on the LRU it has a full reference and thus no longer
needs to be pinned while it is being isolated.

Remove the I_LRU_ISOLATING flag and associated helper functions
(inode_pin_lru_isolating, inode_unpin_lru_isolating, and
inode_wait_for_lru_isolating) as they are no longer needed.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>

....

quoted

@@ -745,34 +742,32 @@ is_uncached_acl(struct posix_acl *acl)
  * I_CACHED_LRU		Inode is cached because it is dirty or isn't shrinkable,
  *			and thus is on the s_cached_inode_lru list.
  *
- * __I_{SYNC,NEW,LRU_ISOLATING} are used to derive unique addresses to wait
- * upon. There's one free address left.
+ * __I_{SYNC,NEW} are used to derive unique addresses to wait upon. There are
+ * two free address left.
  */
 
 enum inode_state_bits {
 	__I_NEW			= 0U,
-	__I_SYNC		= 1U,
-	__I_LRU_ISOLATING	= 2U
+	__I_SYNC		= 1U
 };
 
 enum inode_state_flags_t {
 	I_NEW			= (1U << __I_NEW),
 	I_SYNC			= (1U << __I_SYNC),
-	I_LRU_ISOLATING         = (1U << __I_LRU_ISOLATING),
-	I_DIRTY_SYNC		= (1U << 3),
-	I_DIRTY_DATASYNC	= (1U << 4),
-	I_DIRTY_PAGES		= (1U << 5),
-	I_CLEAR			= (1U << 6),
-	I_LINKABLE		= (1U << 7),
-	I_DIRTY_TIME		= (1U << 8),
-	I_WB_SWITCH		= (1U << 9),
-	I_OVL_INUSE		= (1U << 10),
-	I_CREATING		= (1U << 11),
-	I_DONTCACHE		= (1U << 12),
-	I_SYNC_QUEUED		= (1U << 13),
-	I_PINNING_NETFS_WB	= (1U << 14),
-	I_LRU			= (1U << 15),
-	I_CACHED_LRU		= (1U << 16)
+	I_DIRTY_SYNC		= (1U << 2),
+	I_DIRTY_DATASYNC	= (1U << 3),
+	I_DIRTY_PAGES		= (1U << 4),
+	I_CLEAR			= (1U << 5),
+	I_LINKABLE		= (1U << 6),
+	I_DIRTY_TIME		= (1U << 7),
+	I_WB_SWITCH		= (1U << 8),
+	I_OVL_INUSE		= (1U << 9),
+	I_CREATING		= (1U << 10),
+	I_DONTCACHE		= (1U << 11),
+	I_SYNC_QUEUED		= (1U << 12),
+	I_PINNING_NETFS_WB	= (1U << 13),
+	I_LRU			= (1U << 14),
+	I_CACHED_LRU		= (1U << 15)
 };

This is a bit of a mess - we should reserve the first 4 bits for the
waitable inode_state_bits right from the start and not renumber the
other flag bits into that range. i.e. start the first non-waitable
bit at bit 4. That way every time we add/remove a waitable bit, we
don't have to rewrite the entire set of flags. i.e: something like:

enum inode_state_flags_t {
	I_NEW			= (1U << __I_NEW),
	I_SYNC			= (1U << __I_SYNC),
	// waitable bit 2 unused
	// waitable bit 3 unused
	I_DIRTY_SYNC		= (1U << 4),
....

This will be much more blame friendly if we do it this way from the
start of this patch set.

Thanks. I had this locally a bit differently but I just change it to a
comment.