On Friday 05 November 2004 06:48, Jeff Dike wrote:
> [email protected] said:
> > I had a doubt on this, but I was not getting much feedback from you...
> Yeah, sorry.
Jeff, please read this one - I have a number of important things in it. Also,
I'd like comments from everyone else interested.
I can understand you, so I'll try to reduce my mails to you to increase the
signal/noise ratio.
I'll still send what needed to the list, but CC you just when requesting your
attention (not for trivial fixlet, yes for things as this one, which smelt
like being an hack done on purpose).
> > Also, if you reject this, I'd require a comment-only patch for it: "as
> > soon as I remember why" makes me think back to my yesterday's class,
> > when the teacher said "put comments in your code or you'll soon
> > forget what it does!" 8-O (yes, 1st year University student :-( ).
> The thing is, you often don't realize what's going to be mysterious until
> it actually is, and then it's too late for the comment :-)
> In this case, it wants to be bounced out
You mean it failing with EINTR, right?
> of sigprocmask when a SIGWINCH
> arrives.
Also, why shouldn't sigprocmask be restartable with the -ERESTART* mechanism?
Wouldn't your kludge break?
Also, a nicer way to code this could be to have an explicit sighandler setting
a flag (to get the syscall interrupted if the signal arrives before being
blocked) and to call sigpending() (to test if the signal arrived just after
setting it). After the syscall, that could become SIG_IGN.
Also, (optional answer), why is this needed? A comment about such issues would
be better than an answer email.
> In order to do so, it must have a handler registered, even if
> it does nothing.
Ok. However, I have a general question about all this whole code: why do you
use pipes as synchronization primitives? Did you avoid semaphores for
portability issues, or for persistency ones? Both can be solved (with the os_
layer and the IPC_PRIVATE key).
This would especially help during context switching, I think. I have just a
rough idea of what switch_pipe is for, but calling the network layer (what
you call os_pipe() is actually socketpair(), which is very confusing) to rely
on the semaphores / wait queues it uses seems suboptimal and ugly.
What are your ideas about this?
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
On Fri, Nov 05, 2004 at 08:36:55PM +0100, Blaisorblade wrote:
> Also, why shouldn't sigprocmask be restartable with the -ERESTART* mechanism?
Err, that was pause, not sigprocmask. sigprocmask just switches the signal
mask.
pause is what sits there and waits for something to happen.
> Wouldn't your kludge break?
What kludge?
> Also, a nicer way to code this could be to have an explicit sighandler setting
> a flag (to get the syscall interrupted if the signal arrives before being
> blocked) and to call sigpending() (to test if the signal arrived just after
> setting it). After the syscall, that could become SIG_IGN.
You're seeing races where there aren't any. SIGWINCH is only possible when
it gets a controlling tty, which happens after the sigprocmask.
> Also, (optional answer), why is this needed? A comment about such issues would
> be better than an answer email.
The point of this is to handle SIGWINCH on consoles which have host ttys and
relay them inside UML to whatever might be running on the console and cares
about the window size.
So, we have a separate thread for each host tty attached to a UML device
(side-issue - I'm annoyed that one thread can't have multiple controlling
ttys for purposed of handling SIGWINCH, but I imagine there are other reasons
that doesn't make any sense).
SIGWINCH can't be received synchronously, so you have to set up to receive
it as a signal. That being the case, if you are going to wait for it, it is
convenient to sit in a pause() and wait for the signal to bounce you out of
it. So, you need a signal handler for the reason I mentioned before, but it's
not needed to do anything.
> Ok. However, I have a general question about all this whole code: why do you
> use pipes as synchronization primitives? Did you avoid semaphores for
> portability issues, or for persistency ones? Both can be solved (with the os_
> layer and the IPC_PRIVATE key).
OK, the synchronization here is due to that fact that the argument recieved
by the thread is on the stack of the parent. The parent can't return from
its current procedure until it knows that the child has copied the data onto
its own stack. Hence the writing of the byte after it has been copied.
A semaphore would work, and in fact that's exactly what I'm imitating here.
I've avoided SysV semaphores because of a long-standing dislike of them,
stemming mostly from their persistence, and the fact that they need to be
cleaned up manually if the process that created them didn't destroy them.
I just had a look at the man page, and I see this:
The name choice IPC_PRIVATE was perhaps unfortunate, IPC_NEW would more
clearly show its function.
So, IPC_PRIVATE ones would seem to be persistent, too.
A more basic issue is the interface. What I have now is the mapping
open <-> create
read <-> down
write <-> up
close <-> destroy
which is way simpler and cleaner than semget, semop, and ??? (I can't figure
out how to destroy one of these things).
> This would especially help during context switching, I think. I have just a
> rough idea of what switch_pipe is for, but calling the network layer (what
> you call os_pipe() is actually socketpair(), which is very confusing) to rely
> on the semaphores / wait queues it uses seems suboptimal and ugly.
For context switching, normal pipes (or semaphores) would be fine. I use
socketpairs because they support SIGIO, and pipes don't. I stuck with
socketpairs just because they do everything I need.
Jeff
On Saturday 06 November 2004 06:13, Jeff Dike wrote:
> On Fri, Nov 05, 2004 at 08:36:55PM +0100, Blaisorblade wrote:
> > Also, why shouldn't sigprocmask be restartable with the -ERESTART*
> > mechanism?
> Err, that was pause, not sigprocmask. sigprocmask just switches the signal
> mask.
Ok, I saw it able to return EINTR, so I thought you wanted it to bounce back
there (I wasn't understanding the purpose of the code anyway, now with your
help it's clear).
> pause is what sits there and waits for something to happen.
> > Wouldn't your kludge break?
> What kludge?
[...]
> You're seeing races where there aren't any. SIGWINCH is only possible when
> it gets a controlling tty, which happens after the sigprocmask.
When writing this I was still thinking to sigprocmask() getting SIGWINCH, not
pause. So obviously I said nonsense.
I also understand now what all this is for. When I have time for this, I'll at
least copy and paste your mail into a comment, with any needed adjustment.
For the semaphore issue, I have some ideas (like using futexes) which need to
be developed a bit:
1) I want to create a semaphore API in os_*.
2) It will be able to use socketpairs.
3) It will be able to use futexes, if they are non-persistant and usable
without too much issues (the same way we are going to support Async I/O).
4) It will be used first by the code which could really benefit from the
performance increase.
5) It won't use persistant objects.
Any comment on these issues? Also, apart TT context switching, is there any
other performance-sensitive use of semaphores, which would benefit from using
futexes?
About this:
> A more basic issue is the interface.
> What I have now is the mapping
> open <-> create
> read <-> down
> write <-> up
> close <-> destroy
> which is way simpler and cleaner than semget, semop, and ??? (I can't
> figure out how to destroy one of these things).
Yes, semget and friends are uglier.
But don't think that the current nested code is simple to read - three
semaphores at a time, without a clear name, are not the clearer code on the
world.
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
[email protected] said:
> I also understand now what all this is for. When I have time for this,
> I'll at least copy and paste your mail into a comment, with any
> needed adjustment.
That would be a good idea.
> For the semaphore issue, I have some ideas (like using futexes) which
> need to be developed a bit:
> 1) I want to create a semaphore API in os_*.
> 2) It will be able to use socketpairs.
> 3) It will be able to use futexes, if they are
> non-persistant and usable without too much issues (the same way we
> are going to support Async I/O).
> 4) It will be used first by the code
> which could really benefit from the performance increase.
> 5) It won't
> use persistant objects.
This all sounds good, although there are simplicity benefits to just using
one underlying mechanism, as long as there are no overriding disadvantages
to it.
> Any comment on these issues? Also, apart TT context switching, is
> there any other performance-sensitive use of semaphores, which would
> benefit from using futexes?
Offhand, I think context switching is the most sensitive one.
> Yes, semget and friends are uglier.
> But don't think that the current nested code is simple to read - three
> semaphores at a time, without a clear name, are not the clearer code
> on the world.
What nested code are you talking about?
Jeff
On Tuesday 09 November 2004 21:48, Jeff Dike wrote:
> [email protected] said:
> > I also understand now what all this is for. When I have time for this,
> > I'll at least copy and paste your mail into a comment, with any
> > needed adjustment.
> That would be a good idea.
> > For the semaphore issue, I have some ideas (like using futexes) which
> > need to be developed a bit:
> > 1) I want to create a semaphore API in os_*.
> > 2) It will be able to use socketpairs.
> > 3) It will be able to use futexes, if they are
> > non-persistant and usable without too much issues (the same way we
> > are going to support Async I/O).
> > 4) It will be used first by the code
> > which could really benefit from the performance increase.
> > 5) It won't
> > use persistant objects.
> This all sounds good, although there are simplicity benefits to just using
> one underlying mechanism, as long as there are no overriding disadvantages
> to it.
Yes, I would like that, too, but futexes are 2.6 only, and probably also
NPTL-only (we are going to fix that, at least for SKAS mode), but faster than
anything else. Nothing apart this.
> > Any comment on these issues? Also, apart TT context switching, is
> > there any other performance-sensitive use of semaphores, which would
> > benefit from using futexes?
> Offhand, I think context switching is the most sensitive one.
Ok. But to get TT mode to work against NPTL glibc, which is required for
futexes, we need to recode the "thread_private" section in uml.lds.S to work
with NPTL glibc. It seems that binutils does not like that (the error is "not
enough program header allocated", which refers to the fact that
SIZEOF_HEADERS is guessed and then used. Not using SIZEOF_HEADERS could help,
if doing this is possible).
> > Yes, semget and friends are uglier.
> > But don't think that the current nested code is simple to read - three
> > semaphores at a time, without a clear name, are not the clearer code
> > on the world.
> What nested code are you talking about?
There are two down() and two up(); additionally, run_helper_thread() manages
at least another pipe(). I don't see an easy way to simplifying all this, but
it's needed (or at least some comment should be added). Just a cleanup,
anyway.
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
Blaisorblade wrote:
> Yes, I would like that, too, but futexes are 2.6 only, and probably also
> NPTL-only (we are going to fix that, at least for SKAS mode), but faster than
> anything else. Nothing apart this.
Actually, you can use raw futexes directly without needing any thread library.
There is even some helper code available if you search around a bit.
Chris