Re: [PATCH 04/23] rt2x00: Make rt2x00_queue_entry_for_each more flexible

[Yasushi SHOJI <yashi@atmark-techno.com>]

Hi,

At Mon, 18 Apr 2011 15:27:43 +0200,
Ivo van Doorn wrote:
> 
> From: Helmut Schaa <helmut.schaa-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> Allow passing a void pointer to rt2x00_queue_entry_for_each which in
> turn in provided to the callback function.
> 
> Furthermore, allow the callback function to stop processing by returning
> true. And also notify the caller of rt2x00_queue_entry_for_each if the
> loop was canceled by the callback.

My colleague just tested this patch set and found out that the in
question makes our 400MHz ARM cpu board with ralink usb dongle non
functional due to high cpu consumption.  it seems for us that exiting
from function every time it finds an entry is too expensive on systems
slower than PCs.

To verify our thought, we changed the source code as the patch below.
What we intended to do with this change is to continue processing all
entry without breaking semantics.

With the patch below our board seem to work fine again, but not sure
exactly why it takes so much time to check the list again.  We are not
against the idea of the patch at all.  We just want to ask you guys
how we should go to track this problem. it might be the slow usb?

Thanks,

> diff -urN compat-wireless-2011-04-26/drivers/net/wireless/rt2x00/rt2x00queue.c compat-wireless-2011-04-26-armadillo/drivers/net/wireless/rt2x00/rt2x00queue.c
> --- compat-wireless-2011-04-26/drivers/net/wireless/rt2x00/rt2x00queue.c	2011-04-27 04:04:26.000000000 +0900
> +++ compat-wireless-2011-04-26-armadillo/drivers/net/wireless/rt2x00/rt2x00queue.c	2011-04-27 17:13:45.000000000 +0900
> @@ -740,6 +740,7 @@
>  	unsigned int index_start;
>  	unsigned int index_end;
>  	unsigned int i;
> +	bool ret = false;
>  
>  	if (unlikely(start >= Q_INDEX_MAX || end >= Q_INDEX_MAX)) {
>  		ERROR(queue->rt2x00dev,
> @@ -766,21 +767,21 @@
>  	if (index_start < index_end) {
>  		for (i = index_start; i < index_end; i++) {
>  			if (fn(&queue->entries[i], data))
> -				return true;
> +				ret = true;
>  		}
>  	} else {
>  		for (i = index_start; i < queue->limit; i++) {
>  			if (fn(&queue->entries[i], data))
> -				return true;
> +				ret = true;
>  		}
>  
>  		for (i = 0; i < index_end; i++) {
>  			if (fn(&queue->entries[i], data))
> -				return true;
> +				ret = true;
>  		}
>  	}
>  
> -	return false;
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(rt2x00queue_for_each_entry);
-- 
          yashi