iversion: make inode_cmp_iversion{+raw} return bool instead of s64 (c0cef30e) · Commits · 戴 / test

As Linus points out: The inode_cmp_iversion{+raw}() functions are pure and utter crap. Why? You say that they return 0/negative/positive, but they do so in a completely broken manner. They return that ternary value as the sequence number difference in a 's64', which means that if you actually care about that ternary value, and do the *sane* thing that the kernel-doc of the function implies is the right thing, you would do int cmp = inode_cmp_iversion(inode, old); if (cmp < 0 ... and as a result you get code that looks sane, but that doesn't actually *WORK* right. Since none of the callers actually care about the ternary value here, convert the inode_cmp_iversion{+raw} functions to just return a boolean value (false for matching, true for non-matching). This matches the existing use of these functions just fine, and makes it simple to convert them to return a ternary value in the future if we grow callers that need it. With this change we can also reimplement inode_cmp_iversion in a simpler way using inode_peek_iversion. Signed-off-by:

Jeff Layton <jlayton@redhat.com> Signed-off-by:

Linus Torvalds <torvalds@linux-foundation.org>

include/linux/iversion.h

+8 −12

Original line number	Diff line number	Diff line
		@@ -309,13 +309,13 @@ inode_query_iversion(struct inode *inode)
		* @inode: inode to check
		* @old: old value to check against its i_version
		*
		* Compare the current raw i_version counter with a previous one. Returns 0 if
		* they are the same or non-zero if they are different.
		* Compare the current raw i_version counter with a previous one. Returns false
		* if they are the same or true if they are different.
		*/
		static inline s64
		static inline bool
		inode_cmp_iversion_raw(const struct inode *inode, u64 old)
		{
		return (s64)inode_peek_iversion_raw(inode) - (s64)old;
		return inode_peek_iversion_raw(inode) != old;
		}

		/**
		@@ -323,19 +323,15 @@ inode_cmp_iversion_raw(const struct inode *inode, u64 old)
		* @inode: inode to check
		* @old: old value to check against its i_version
		*
		* Compare an i_version counter with a previous one. Returns 0 if they are
		* the same, a positive value if the one in the inode appears newer than @old,
		* and a negative value if @old appears to be newer than the one in the
		* inode.
		* Compare an i_version counter with a previous one. Returns false if they are
		* the same, and true if they are different.
		*
		* Note that we don't need to set the QUERIED flag in this case, as the value
		* in the inode is not being recorded for later use.
		*/

		static inline s64
		static inline bool
		inode_cmp_iversion(const struct inode *inode, u64 old)
		{
		return (s64)(inode_peek_iversion_raw(inode) & ~I_VERSION_QUERIED) -
		(s64)(old << I_VERSION_QUERIED_SHIFT);
		return inode_peek_iversion(inode) != old;
		}
		#endif

Admin message