Re: [PATCH] -mm: i386 apm.c optimization

[Bill Davidsen <davidsen@tmr.com>]

Andreas Mohr wrote:

>Hello all,
>
>- avoid expensive modulo (integer division) which happened
>  since APM_MAX_EVENTS is 20 (non-power-of-2)
>- kill compiler warnings by initializing two variables
>- add __read_mostly to some important static variables that are read often
>  (by idle loop etc.)
>- constify several structures
>
>Patch tested on 2.6.16-ck5, rediffed against 2.6.17-rc1-mm2. 
>@@ -1104,7 +1105,8 @@
> 
> static apm_event_t get_queued_event(struct apm_user *as)
> {
>-	as->event_tail = (as->event_tail + 1) % APM_MAX_EVENTS;
>+	if (++as->event_tail >= APM_MAX_EVENTS)
>+		as->event_tail = 0;
> 	return as->events[as->event_tail];
> }
> 
>  
>
Either event_tail can never be > APM_MAX_EVENTS (I believe that's true) 
and you should use ==, or you should do a proper mod function:
  ++as->event_tail;
  while (as->event_tail >= APM_MAX_EVENTS) as->event_tail -= APM_MAX_EVENTS;

In the unlikely even that the event_tail is already too large you want a 
proper mod, not to set it to zero.

-- 
bill davidsen <davidsen@tmr.com>
  CTO TMR Associates, Inc
  Doing interesting things with small computers since 1979