Re: [PATCH] sched: activate SCHED BATCH expired

[Con Kolivas <kernel@kolivas.org>]

On Saturday 18 March 2006 00:26, Nick Piggin wrote:
> Con Kolivas wrote:
> > -static inline void __activate_task(task_t *p, runqueue_t *rq)
> > +static void __activate_task(task_t *p, runqueue_t *rq)
> >  {
> > -	enqueue_task(p, rq->active);
> > +	if (batch_task(p))
> > +		enqueue_task(p, rq->expired);
> > +	else
> > +		enqueue_task(p, rq->active);
> >  	inc_nr_running(p, rq);
> >  }
>
> I prefer:
>
>    prio_array_t *target = rq->active;
>    if (batch_task(p))
>      target = rq->expired;
>    enqueue_task(p, target);
>
> Because gcc can use things like predicated instructions for it.
> But perhaps it is smart enough these days to recognise this?
> At least in the past I have seen it start using cmov after doing
> such a conversion.
>
> At any rate, I think it looks nicer as well. IMO, of course.

Well on my one boring architecture here is a before and after, gcc 4.1.0 with 
optimise for size kernel config:
0xb01127da <__activate_task+0>: push   %ebp
0xb01127db <__activate_task+1>: mov    %esp,%ebp
0xb01127dd <__activate_task+3>: push   %esi
0xb01127de <__activate_task+4>: push   %ebx
0xb01127df <__activate_task+5>: mov    %eax,%esi
0xb01127e1 <__activate_task+7>: mov    %edx,%ebx
0xb01127e3 <__activate_task+9>: cmpl   $0x3,0x58(%eax)
0xb01127e7 <__activate_task+13>:        jne    0xb01127ee <__activate_task+20>
0xb01127e9 <__activate_task+15>:        mov    0x44(%edx),%edx
0xb01127ec <__activate_task+18>:        jmp    0xb01127f1 <__activate_task+23>
0xb01127ee <__activate_task+20>:        mov    0x40(%edx),%edx
0xb01127f1 <__activate_task+23>:        mov    %esi,%eax
0xb01127f3 <__activate_task+25>:        call   0xb01124bb <enqueue_task>
0xb01127f8 <__activate_task+30>:        incl   0x8(%ebx)
0xb01127fb <__activate_task+33>:        mov    0x18(%esi),%eax
0xb01127fe <__activate_task+36>:        add    %eax,0xc(%ebx)
0xb0112801 <__activate_task+39>:        pop    %ebx
0xb0112802 <__activate_task+40>:        pop    %esi
0xb0112803 <__activate_task+41>:        pop    %ebp
0xb0112804 <__activate_task+42>:        ret

Your version:
0xb01127da <__activate_task+0>: push   %ebp
0xb01127db <__activate_task+1>: mov    %esp,%ebp
0xb01127dd <__activate_task+3>: push   %esi
0xb01127de <__activate_task+4>: push   %ebx
0xb01127df <__activate_task+5>: mov    %eax,%esi
0xb01127e1 <__activate_task+7>: mov    %edx,%ebx
0xb01127e3 <__activate_task+9>: mov    0x40(%edx),%edx
0xb01127e6 <__activate_task+12>:        cmpl   $0x3,0x58(%eax)
0xb01127ea <__activate_task+16>:        jne    0xb01127ef <__activate_task+21>
0xb01127ec <__activate_task+18>:        mov    0x44(%ebx),%edx
0xb01127ef <__activate_task+21>:        mov    %esi,%eax
0xb01127f1 <__activate_task+23>:        call   0xb01124bb <enqueue_task>
0xb01127f6 <__activate_task+28>:        incl   0x8(%ebx)
0xb01127f9 <__activate_task+31>:        mov    0x18(%esi),%eax
0xb01127fc <__activate_task+34>:        add    %eax,0xc(%ebx)
0xb01127ff <__activate_task+37>:        pop    %ebx
0xb0112800 <__activate_task+38>:        pop    %esi
0xb0112801 <__activate_task+39>:        pop    %ebp
0xb0112802 <__activate_task+40>:        ret

I'm not attached to the style, just the feature. If you think it's warranted 
I'll change it.

Cheers,
Con
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/