On Tuesday 21 March 2006 02:00, Jeff Dike wrote:
> On Fri, Mar 17, 2006 at 08:36:58PM +0100, Blaisorblade wrote:
> > 2) write_sigio_thread should do a "down" on a semaphore/mutex and the
> > first update_thread should "up" it. As usually, this semaphore would be
> > indeed implemented as a pipe.
> Is there anything wrong with this:
> Index: linux-2.6.15/arch/um/os-Linux/sigio.c
> ===================================================================
> --- linux-2.6.15.orig/arch/um/os-Linux/sigio.c 2006-03-20
> 19:48:51.000000000 -0500 +++
> linux-2.6.15/arch/um/os-Linux/sigio.c 2006-03-20 19:49:55.000000000 -0500
> @@ -271,6 +271,10 @@ void write_sigio_workaround(void)
> if(write_sigio_pid != -1)
> goto out_unlock;
>
> + current_poll = ((struct pollfds) { .poll = p,
> + .used = 1,
> + .size = 1 });
> +
> write_sigio_pid = run_helper_thread(write_sigio_thread, NULL,
> CLONE_FILES | CLONE_VM, &stack, 0);
>
> @@ -284,10 +288,6 @@ void write_sigio_workaround(void)
> memcpy(write_sigio_fds, l_write_sigio_fds, sizeof(l_write_sigio_fds));
> memcpy(sigio_private, l_sigio_private, sizeof(l_sigio_private));
>
> - current_poll = ((struct pollfds) { .poll = p,
> - .used = 1,
> - .size = 1 });
> -
> sigio_unlock();
> return;
>
> That seems to me to fix the basic problem - the thread is using data
> that hasn't been set up yet, and I don't see that moving it up breaks
> anything.
> Jeff
Ok, this is conceptually correct, and looking at 2.6.14 code this bug wasn't
there, so _I_ introduced it, while cleaning it up. Sorry. Beyond, my idea of
adding the semaphore was incorrect (it would work only if
write_sigio_workaround did the up()).
However, while I overlooked this, you overlooked what I did:
*) you must also move the setting of write_sigio_fds and sigio_private above,
or we'll get the same problem again in different places of
write_sigio_thread.
*) you must update the exit path. I've taken care so that only if everything
succeed the result is stored, and to hold the lock for the minimum time
needed (particularly not while doing any allocation). I'm not sure whether
the first property is needed, but it would be conservative to do so to
preserve coherence of data structures.
In particular: if we store an fd != -1, some function may later close it (say
when the console is closed); if we closed it in the exit path, then we're
closing an unrelated fd and getting a bug.
So, if the thread startup fails, you _must_ clear again current_poll,
write_sigio_fds and sigio_private.
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade
___________________________________
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB
http://mail.yahoo.it
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
