[9fans] 9front sleep interrupted in kproc?

[9fans] 9front sleep interrupted in kproc?

[erik quanstrom]

not sure where to send this comment.

i think this patch misunderstands the situation.  the patch
claims that some code is wrong because sleep in a kproc might
get interrupted.

	http://code.google.com/p/plan9front/source/detail?r=3864ff1fe83f254622e6f10925f53df62255336d

this diff highlights the issue

	http://code.google.com/p/plan9front/source/diff?spec=svn3864ff1fe83f254622e6f10925f53df62255336d&r=3864ff1fe83f254622e6f10925f53df62255336d&format=side&path=/sys/src/9/port/alarm.c

the crux of the matter is when sleep might be interrupted.  sleep
is interrupted iff process received a note.

since kprocs don't get notes (it's an error to write to the note file),
sleep in a kproc can't get interrupted.  this code will never fire.

- erik

Re: [9fans] 9front sleep interrupted in kproc?

[Anthony Martin]

erik quanstrom <quanstro@quanstro.net> once said:
> since kprocs don't get notes (it's an error to write to the note file),
> sleep in a kproc can't get interrupted.  this code will never fire.

It looks like they can on 9front.

The following is from pc/vgavesa.c:/^vesaproc

	/*
	 * dont wait forever here. some BIOS get stuck
	 * in i/o poll loop after blank/unblank for some
	 * reason. (Thinkpad A22p)
	 */
	procalarm(10000);

Cheers,
  Anthony

Re: [9fans] 9front sleep interrupted in kproc?

[Anthony Martin]

Anthony Martin <ality@pbrane.org> once said:
> erik quanstrom <quanstro@quanstro.net> once said:
> > since kprocs don't get notes (it's an error to write to the note file),
> > sleep in a kproc can't get interrupted.  this code will never fire.
>
> It looks like they can on 9front.

Actually, this isn't just 9front. All of the network
medium receive kprocs (e.g., etherread4) can be sent
notes (only by the kernel, of course).

  Anthony

Re: [9fans] 9front sleep interrupted in kproc?

[erik quanstrom]

On Mon Dec 30 20:26:47 EST 2013, ality@pbrane.org wrote:
> Anthony Martin <ality@pbrane.org> once said:
> > erik quanstrom <quanstro@quanstro.net> once said:
> > > since kprocs don't get notes (it's an error to write to the note file),
> > > sleep in a kproc can't get interrupted.  this code will never fire.
> >
> > It looks like they can on 9front.
>
> Actually, this isn't just 9front. All of the network
> medium receive kprocs (e.g., etherread4) can be sent
> notes (only by the kernel, of course).

there are no calls to procalarm() outside of sysalarm (the alarm
system call) in either the official sources or 9atom (either the 64bit
or regular kernels).

if i've pulled correctly, 9front has one procalarm, and it's in the vesa code
with a waserror at the ready.

this patch addresses a problem that doesn't exist.  it seems to imply
that notes can come to kprocs out of the blue.  and i believe this is
not correct.

- erik

Re: [9fans] 9front sleep interrupted in kproc?

[Charles Forsyth]

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

On 31 December 2013 01:24, Anthony Martin <ality@pbrane.org> wrote:

> All of the network
> medium receive kprocs (e.g., etherread4) can be sent
> notes (only by the kernel, of course).
>

yes, but they allow for that.

[-- Attachment #2: Type: text/html, Size: 558 bytes --]

Re: [9fans] 9front sleep interrupted in kproc?

[cinap_lenrek]

its just a case of defensive programming paranoia. i did the change
after sl reported wifi kproc exiting (went into Broken) state. there
seemed no other explaination other than a note interrupting it
so i made all the kprocs safe to be interrupted by notes. i found this
pattern in many places. most of the kprocs are just loops waiting for
something and in case of error, just restart the loop. even if it is
obvious that nobody is calling error() in there. (see devfloppy
kproc for example). you just setup the error label at before your loop
and it make it automatically restart your loop on errors. this doesnt
cost you any cycles in the loop. when i see this code, i can stop worrying
and assume *less* about of the surroundings.

so simply put, yes, you are right. these kprocs shouldnt get interrupted.
but i made sure to catch the case anyway.

i wouldnt rule out notes out of the blue. the use of postnote() is racy in
many places. a Proc* pointer is not a good identifier for a process
*unless* you can guaratee that the proc will not exit while you call
postnote(). alarms have this race. pexit() just does up->alarm = 0;
to clear alarms. if the alarm kproc was already commited
posting you the note at this point (and before it got hold onto
the p->debug qlock), the new process reusing your Proc*
structure can get the alarm out of the blue.

--
cinap

Re: [9fans] 9front sleep interrupted in kproc?

[erik quanstrom]

On Tue Dec 31 08:19:04 EST 2013, cinap_lenrek@felloff.net wrote:
> its just a case of defensive programming paranoia. i did the change
> after sl reported wifi kproc exiting (went into Broken) state. there
> seemed no other explaination other than a note interrupting it
> so i made all the kprocs safe to be interrupted by notes. i found this
> pattern in many places. most of the kprocs are just loops waiting for
> something and in case of error, just restart the loop. even if it is
> obvious that nobody is calling error() in there. (see devfloppy
> kproc for example). you just setup the error label at before your loop
> and it make it automatically restart your loop on errors. this doesnt
> cost you any cycles in the loop. when i see this code, i can stop worrying
> and assume *less* about of the surroundings.
>
> so simply put, yes, you are right. these kprocs shouldnt get interrupted.
> but i made sure to catch the case anyway.
>
> i wouldnt rule out notes out of the blue. the use of postnote() is racy in
> many places. a Proc* pointer is not a good identifier for a process
> *unless* you can guaratee that the proc will not exit while you call
> postnote(). alarms have this race. pexit() just does up->alarm = 0;
> to clear alarms. if the alarm kproc was already commited
> posting you the note at this point (and before it got hold onto
> the p->debug qlock), the new process reusing your Proc*
> structure can get the alarm out of the blue.

thanks for the discussion.

in my view, there is a point where defensive programming against
can't-happen events becomes counter productive.  first, the code can't
be tested, since it can't happen.  (can the kproc be restarted.  quite a
few can't.)  second, it leads the poor reader (and there are always more
readers than writers) to believe the event could happen, when it can't.
third, were it to happen by some newly introduced bug, the obvious
manifestation might be hidden by error recovery.

in short, defending against things that can't happen is imho an anti-pattern.
i don't mean things like users or other programs providing bad input,
or things that are not proveably correct, but the baseline invarients.
i think the ip code gets it right.  it panics if the ip version is ever
outside {V4, V6}.

out-of-the-blue notes can't happen without a serious bug.  were that
to happen (and we have no evidence that it has), shouldn't that be
fixed instead?

did you find the wifi issue?

- erik