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:
> > 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@xxxxxxxxxxxxxx>
>
> ....
> > @@ -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.
