Re: [PATCH] Significant performace improvements on reiserfs systems

Andrea Arcangeli
2001-09-20T23:15:35Z

On Thu, Sep 20, 2001 at 06:56:04PM -0400, Robert Love wrote:
> On Thu, 2001-09-20 at 18:37, Andrea Arcangeli wrote:
> > On Thu, Sep 20, 2001 at 06:24:48PM -0400, Robert Love wrote:
> > >
> > > if (current->need_resched && current->lock_depth == 0) {
> > > unlock_kernel();
> > > lock_kernel();
> > > }
>
> > nitpicking: the above is fine but it isn't complete, it may work for
> > most cases but for a generic function it would be better implemented
> > similarly to release_kernel_lock_save/restore so you take care of
> > lock_depth > 0 too:
>
> Let me explain a little about the patch, and then I am interested in if
> your opinion changes.
>
> When unlock_kernel() is called, the preemption code will be enabled and
> check if the preemption count is non-zero -- its handled just like a

All I'm saying is that you should check for >= 0, not == 0.

But anwyays it's pretty depressing to see such a costly check needed to
get latency right with the preemptive kernel approch, with
non-preemptive kernel we'd need to just check need_resched and a call
schedule in the unlikely case so it would be even lighter :) and no
fixed costs in UP spinlocks, per-cpu datastrctures etc... The point of
preemptive kernel would be just to prevent us to put such kind of
explicit (costly) checks in the code. My theory was that the cpu-costly
loops are mostly protected by some lock anyways and the fact you're
writing such horrid (but helpful) code is a kind of proof.

Andrea