[9fans] run queues

[9fans] run queues

[Philippe Anel]

Hi 9fans,

While reading src/9/port/proc.c:^runproc, I realized that the
following code is called without any lock. I would expect one in order
to walk through the each rq lists, as it is called with interrupt
enabled.

I think this would not crash the system because procs are not
dynamically allocated or freed.

I also know that locking can be a problem regarding performance.

        for(rq = &runq[Nrq-1]; rq >= runq; rq--){
            for(p = rq->head; p; p = p->rnext){
                if(p->mp == nil || p->mp == MACHP(m->machno)
                || (!p->wired && i > 0))
                    goto found;
            }
        }

What do you think ?

Phil;

Ps: It seems runqs were designed to be individually locked. Is there
any info about the reason why all the runq are locked at once using :

    lock(runq);
    // ...
    unlock(runq);

where we could expect (with some other changes of course) :

    lock(rq);
    // ...
    unlock(rq);

?

[9fans] run queues

[Sape Mullender]

Well spotted.  But the queue is maintained in a way that
traversing it while it's being changed cannot cause one
to address garbage (rnext always contains a pointer to a
proc or nil) and, at the label found, the sanity check
is performed to see if we really have a  proc on the queue
in our hands.  Worst case is that the search terminates too
early, but then we just stay in the search loop.

	Sape


> While reading /sys/src/9/port/proc.c:^runproc, I realized that the
> following code is called without any lock. I would expect one in order
> to walk through the each rq lists, as it is called with interrupt
> enabled.
>
> I think this would not crash the system because procs are not
> dynamically allocated or freed.
>
> I also know that locking can be a problem regarding performance.
>
>  for(rq = &runq[Nrq-1]; rq >= runq; rq--){
>  for(p = rq->head; p; p = p->rnext){
>  if(p->mp == nil || p->mp == MACHP(m->machno)
>  || (!p->wired && i > 0))
>  goto found;
>  }
>  }
>
> What do you think ?

Re: [9fans] run queues

[Philippe Anel]

Thank you. That's what I was thinking. I suppose this has been written
like this because traversing the queue might require too much time for
having a lock.

Phil;

Sape Mullender wrote:
> Well spotted.  But the queue is maintained in a way that
> traversing it while it's being changed cannot cause one
> to address garbage (rnext always contains a pointer to a
> proc or nil) and, at the label found, the sanity check
> is performed to see if we really have a  proc on the queue
> in our hands.  Worst case is that the search terminates too
> early, but then we just stay in the search loop.
>
> 	Sape
>
>
>
>> While reading /sys/src/9/port/proc.c:^runproc, I realized that the
>> following code is called without any lock. I would expect one in order
>> to walk through the each rq lists, as it is called with interrupt
>> enabled.
>>
>> I think this would not crash the system because procs are not
>> dynamically allocated or freed.
>>
>> I also know that locking can be a problem regarding performance.
>>
>>  for(rq = &runq[Nrq-1]; rq >= runq; rq--){
>>  for(p = rq->head; p; p = p->rnext){
>>  if(p->mp == nil || p->mp == MACHP(m->machno)
>>  || (!p->wired && i > 0))
>>  goto found;
>>  }
>>  }
>>
>> What do you think ?
>>
>
>
>

Re: [9fans] run queues

[Sape Mullender]

Yes, that's right.

	Sape


> From: xigh@bouyapop.org
> To: 9fans@9fans.net
> Reply-To: 9fans@9fans.net
> Date: Thu Jun  3 09:15:42 CES 2010
> Subject: Re: [9fans] run queues
>
>
> Thank you. That's what I was thinking. I suppose this has been written
> like this because traversing the queue might require too much time for
> having a lock.
>
> Phil;
>
> Sape Mullender wrote:
> > Well spotted. But the queue is maintained in a way that
> > traversing it while it's being changed cannot cause one
> > to address garbage (rnext always contains a pointer to a
> > proc or nil) and, at the label found, the sanity check
> > is performed to see if we really have a proc on the queue
> > in our hands. Worst case is that the search terminates too
> > early, but then we just stay in the search loop.
> >
> > 	Sape
> >
> >
> >
> >> While reading /sys/src/9/port/proc.c:^runproc, I realized that the
> >> following code is called without any lock. I would expect one in order
> >> to walk through the each rq lists, as it is called with interrupt
> >> enabled.
> >>
> >> I think this would not crash the system because procs are not
> >> dynamically allocated or freed.
> >>
> >> I also know that locking can be a problem regarding performance.
> >>
> >> for(rq = &runq[Nrq-1]; rq >= runq; rq--){
> >> for(p = rq->head; p; p = p->rnext){
> >> if(p->mp == nil || p->mp == MACHP(m->machno)
> >> || (!p->wired && i > 0))
> >> goto found;
> >> }
> >> }
> >>
> >> What do you think ?
> >>
> >
> >
> >
>