On 04/13/2015 10:47 AM, Peter Zijlstra wrote:
On Thu, Apr 09, 2015 at 05:41:44PM -0400, Waiman Long wrote:
+void __init __pv_init_lock_hash(void)
+{
+ int pv_hash_size = 4 * num_possible_cpus();
+
+ if (pv_hash_size< (1U<< LFSR_MIN_BITS))
+ pv_hash_size = (1U<< LFSR_MIN_BITS);
+ /*
+ * Allocate space from bootmem which should be page-size aligned
+ * and hence cacheline aligned.
+ */
+ pv_lock_hash = alloc_large_system_hash("PV qspinlock",
+ sizeof(struct pv_hash_bucket),
+ pv_hash_size, 0, HASH_EARLY,
+ &pv_lock_hash_bits, NULL,
+ pv_hash_size, pv_hash_size);
pv_taps = lfsr_taps(pv_lock_hash_bits);
I don't understand what you meant here.
Let me explain (even though I propose taking all the LFSR stuff out).
pv_lock_hash_bit is a runtime variable, therefore it cannot compile time
evaluate the forest of if statements required to compute the taps value.
Therefore its best to compute the taps _once_, and this seems like a
good place to do so.
OK, I got it. That make sense.
+ goto done;
+ }
+ }
+
+ hash = lfsr(hash, pv_lock_hash_bits, 0);
Since pv_lock_hash_bits is a variable, you end up running through that
massive if() forest to find the corresponding tap every single time. It
cannot compile-time optimize it.
The minimum bits size is now 8. So unless the system has more than 64 vCPUs,
it will get the right value in the first if statement.
Still, no reason to not pre-compute the taps value, its simple enough.
Still, we need to keep the hash_bits value as it will needed by the
hashing function.
I have taken out the lfsr code and use linear probing in the updated
qspinlock patch that I am working on. However, we can always add that
back in as an additional patch.
Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
