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

Cyril Bur [off-list ref] writes:

On Mon, 2017-07-17 at 21:30 +1000, Balbir Singh wrote:

quoted

On Wed, 2017-07-12 at 14:23 +1000, Cyril Bur wrote:

quoted

 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