* [9fans] run queues
@ 2010-06-03 6:43 Philippe Anel
2010-06-03 7:01 ` Sape Mullender
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Anel @ 2010-06-03 6:43 UTC (permalink / raw)
To: Fans of the OS Plan 9 from Bell Labs
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);
?
^ permalink raw reply [flat|nested] 4+ messages in thread
* [9fans] run queues
2010-06-03 6:43 [9fans] run queues Philippe Anel
@ 2010-06-03 7:01 ` Sape Mullender
2010-06-03 7:15 ` Philippe Anel
0 siblings, 1 reply; 4+ messages in thread
From: Sape Mullender @ 2010-06-03 7:01 UTC (permalink / raw)
To: 9fans; +Cc: 9fans
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 ?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [9fans] run queues
2010-06-03 7:01 ` Sape Mullender
@ 2010-06-03 7:15 ` Philippe Anel
2010-06-03 8:12 ` Sape Mullender
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Anel @ 2010-06-03 7:15 UTC (permalink / raw)
To: Fans of the OS Plan 9 from Bell Labs
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 ?
>>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [9fans] run queues
2010-06-03 7:15 ` Philippe Anel
@ 2010-06-03 8:12 ` Sape Mullender
0 siblings, 0 replies; 4+ messages in thread
From: Sape Mullender @ 2010-06-03 8:12 UTC (permalink / raw)
To: 9fans; +Cc: 9fans
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 ?
> >>
> >
> >
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-06-03 8:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-03 6:43 [9fans] run queues Philippe Anel
2010-06-03 7:01 ` Sape Mullender
2010-06-03 7:15 ` Philippe Anel
2010-06-03 8:12 ` Sape Mullender
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).