Messages in this thread | | | Date | Mon, 13 Apr 2015 11:45:00 -0400 | From | Waiman Long <> | Subject | Re: [PATCH v15 09/15] pvqspinlock: Implement simple paravirt support for the qspinlock |
| |
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
| |