Re: [PATCH v2 02/54] fs: add an icount_read helper
From: Mateusz Guzik <hidden>
Date: 2025-08-26 22:18:47
Also in:
linux-btrfs, linux-fsdevel, linux-xfs
On Tue, Aug 26, 2025 at 11:39:02AM -0400, Josef Bacik wrote:
quoted hunk ↗ jump to hunk
Instead of doing direct access to ->i_count, add a helper to handle this. This will make it easier to convert i_count to a refcount later.diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index 079b868552c2..46bfc543f946 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c@@ -66,7 +66,7 @@ static void fsnotify_unmount_inodes(struct super_block *sb) * removed all zero refcount inodes, in any case. Test to * be sure. */ - if (!atomic_read(&inode->i_count)) { + if (!icount_read(inode)) { spin_unlock(&inode->i_lock); continue; }
[snip]
+static inline int icount_read(const struct inode *inode)
+{
+ return atomic_read(&inode->i_count);
+}
+
extern void iget_failed(struct inode *);
extern void clear_inode(struct inode *);
extern void __destroy_inode(struct inode *);The placement issue I mentioned in another e-mail aside, I would recommend further error-proofing this. Above I quoted an example user which treats i_count == 0 as special. While moving this into helpers is definitely a step in the right direction, I think having consumer open-code this check is avoidably error-prone. Notably, as is there is nothing to indicate whether the consumer expects the value to remain stable or is perhaps doing a quick check for other reasons. As such, specific naming aside, I would create 2 variants: 1. icount_read_unstable() -- the value can change from under you arbitrarily. I don't there are any consumers for this sucker atm. 2. icount_read() -- the caller expects the transition 0<->1 is guaranteed to not take place, notably if the value is found to be 0, it stay at 0. to that end the caller is expected to hold the inode spinlock *and* the fact that the lock is held is asserted on with lockdep. All that aside, I think open-coding "is the inode unused" with an explicit count check is bad form -- a dedicated helper for that would also be nice. My 3 CZK.