bfs: better filter invalid timestamp values

Timestamps in BFS are stored with a 16 bit subsecond precision. However,
a lot of files have this field set to 0 as they are created with APIs
that do not set a subsecond precsion value.

The previous format (introduced in hrev31057) did not track wether the
stored data was a 12 bit sequential number or a 12 bit subsecond
timestamp + 4 bit sequential number.

As a result, setting a timestamp with nsec=0 would read back a different
value (exposing the sequential counter to userspace).

Older versions of Haiku, and BeOS according to the Practical Filesystem
Design book, did not store a timestamp there. Haiku used to store the
volume UID (I don't understand why), and BeOS stored a sequence number
because the POSIX APIs at the time didn't allow to get and set the time
with nanosecond precision.

The Practical Filesystem Design already mentions that storing a
subsecond precision value here would have been better than their
workaround.

The new format relies on the fact that nanoseconds can only be in the
range from 0 to 999999999. After the conversion to a 16 bit value using
simple shifts, this result in a maximum possible value of 0xEE60. The
new format exploits that, and uses the range 0xF000 to 0xFFFF to
represent 0 timestamps with a 12 bit sequence number.

This allows the sequence number to be hidden from userspace entirely.

Fixes #19213.

Change-Id: Id80289a4331ecfbf2f1a35520b58a99227d1f1d6
Reviewed-on: https://review.haiku-os.org/c/haiku/+/10146
Tested-by: Commit checker robot <[email protected]>
Reviewed-by: waddlesplash <[email protected]>

src/add-ons/kernel/file_systems/bfs/bfs.h

@@ -246,10 +246,17 @@ struct bfs_inode {
 
 	static time_t ToSecs(int64 time)
 		{ return time >> INODE_TIME_SHIFT; }
-	static uint32 ToNsecs(int64 time)
-		{ return (time & INODE_TIME_MASK) << 14; }
+	static uint32 ToNsecs(int64 time) {
 		// the 16 bits internal resolution shifted by 14 gives us 2^30
 		// which is roughly 10^9, the maximum value in nanoseconds
+		// The maximum value actually used will be 0xEE6B, so the range 0xF000-0xFFFF is used
+		// to represent non-timestamps (pseudorandom values used when the nanosecond part of the
+		// timestamp is 0, to reduce hash collisions for identical timestamps in query index
+		// tables)
+		if ((time & 0xF000) == 0xF000)
+			return 0;
+		return (time & INODE_TIME_MASK) << 14;
+	}
 } _PACKED;
 
 enum inode_flags {
@@ -285,12 +292,20 @@ struct file_cookie {
 #define INODE_NOTIFICATION_INTERVAL	1000000LL
 
 
-/*!	Converts the nano seconds given to the internal 16 bit resolution that
-	BFS uses. If \a time is zero, 12 bits will get a monotonically increasing
-	number. For all other values, only the lower 4 bits are changed this way.
+/*!	Converts the nano seconds given to the internal 16 bit representation that
+	BFS uses.
 
-	This is done to decrease the number of duplicate time values, which speeds
-	up the way BFS handles the time indices.
+	The original BFS in BeOS only reported a resolution of 1 second to userspace (as POSIX APIs at
+	the time didn't allow for more precision). Haiku modified this to allow for subsecond
+	timestamps.
+
+	However, a lot of code may still set times using APIs with a second resolution. This would
+	lead to a lot of similar or identical timestamps, which result in hash collisions in index
+	tables, and as such, bad performance for queries using modification time.
+
+	To avoid this, the lower 4 bits for non-zero timestamps and the lower 12 bits for zero
+	timestamps are replaced with a monotonically increasing number. That makes sure the values
+	are more evenly distributed between different hash buckets.
 */
 inline uint32
 unique_from_nsec(uint32 time)
@@ -299,7 +314,7 @@ unique_from_nsec(uint32 time)
 	if (time != 0)
 		return (((time + 16383) >> 14) & INODE_TIME_MASK) | (++number & 0xf);
 
-	return ++number & 0xfff;
+	return (++number & 0xfff) | 0xf000;
 }