[PATCH v3 06/10] powerpc/opal: Rework the opal-async interface

[Michael Ellerman]

Cyril Bur <cyrilbur at gmail.com> writes:
> On Mon, 2017-07-17 at 21:30 +1000, Balbir Singh wrote:
>> On Wed, 2017-07-12 at 14:23 +1000, Cyril Bur wrote:
>> >  static int __opal_async_get_token(void)
>> >  {
>> >  	unsigned long flags;
>> >  	int token;
>> >  
>> > -	spin_lock_irqsave(&opal_async_comp_lock, flags);
>> > -	token = find_first_bit(opal_async_complete_map, opal_max_async_tokens);
>> > -	if (token >= opal_max_async_tokens) {
>> > -		token = -EBUSY;
>> > -		goto out;
>> > -	}
>> > -
>> > -	if (__test_and_set_bit(token, opal_async_token_map)) {
>> > -		token = -EBUSY;
>> > -		goto out;
>> > +	for (token = 0; token < opal_max_async_tokens; token++) {
>> > +		spin_lock_irqsave(&opal_async_comp_lock, flags);
>> 
>> Why is the spin lock inside the for loop? If the last token is free, the
>> number of times we'll take and release a lock is extensive, why are we
>> doing it this way?
>
> Otherwise we might hold the lock for quite some time.

No we won't. A loop over a few 10s or 100s of tokens is not going to
take long, compared to the overhead of taking the lock every iteration
through the loop.

If the linear search with the lock held is a bottle neck, then you can
keep a hint variable which tracks the last freed token and start
searching from there.

cheers