From: Paolo 'Blaisorblade' Giarrusso <[EMAIL PROTECTED]> To avoid calling request_irq under a spinlock, and to simplify code around, code a state machine to allow safely dropping and retaking sigio_lock during initialization. The state variable is protected by a spinlock together with much other stuff (so there's no reason to use atomic_t). See the long comment for further details.
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[EMAIL PROTECTED]> --- arch/um/os-Linux/sigio.c | 74 +++++++++++++++++++++++++++++----------------- 1 files changed, 46 insertions(+), 28 deletions(-) diff --git a/arch/um/os-Linux/sigio.c b/arch/um/os-Linux/sigio.c index 88988fb..44a2d1f 100644 --- a/arch/um/os-Linux/sigio.c +++ b/arch/um/os-Linux/sigio.c @@ -29,6 +29,24 @@ * exitcall. */ static int write_sigio_pid = -1; +/* State machine: + * + * write_sigio_workaround transitions from INACTIVE to STARTING; all methods + * exit when they detect the STARTING state, so it's effectively a lock without + * wait possibility. After initialization completes it goes to ACTIVE. + * + * Errors in initialization or in use (say the thread does not answer) move us + * to the BROKEN terminal state; this is a change (we went to INACTIVE), and it + * could be reverted so that we go back to INACTIVE, if this case is handled + * correctly without leaks. + * + * closing the device moves us to the CLOSED terminal state. + * + * The state must be read or written only with sigio_lock() held, even when we + * are in STARTING, to ensure global visibility across CPUs. + */ +enum {INACTIVE, STARTING, ACTIVE, BROKEN, CLOSED}; +static int write_sigio_state = INACTIVE; /* These arrays are initialized before the sigio thread is started, and * the descriptors closed after it is killed. So, it can't see them change. @@ -121,7 +139,7 @@ static int need_poll(struct pollfds *polls, int n) } /* Must be called with sigio_lock held, because it's needed by the marked - * critical section. + * critical section. The lock could be moved inside, but it is not needed. */ static void update_thread(void) { @@ -146,6 +164,9 @@ static void update_thread(void) if(write_sigio_pid != -1) os_kill_process(write_sigio_pid, 1); write_sigio_pid = -1; + /* Since the deinit is not complete, we cannot move into INACTIVE state. + * Or can we? */ + write_sigio_state = BROKEN; close(sigio_private[0]); close(sigio_private[1]); close(write_sigio_fds[0]); @@ -199,7 +220,7 @@ int ignore_sigio_fd(int fd) * sigio_cleanup has already run, then update_thread will hang * or fail because the thread is no longer running. */ - if(write_sigio_pid == -1) + if(write_sigio_state != ACTIVE) return -EIO; flags = sigio_lock(); @@ -246,59 +267,53 @@ static void write_sigio_workaround(void) unsigned long stack; struct pollfd *p; int err; - int l_write_sigio_fds[2]; - int l_sigio_private[2]; - int l_write_sigio_pid; unsigned long flags; /* We call this *tons* of times - and most ones we must just fail. */ flags = sigio_lock(); - l_write_sigio_pid = write_sigio_pid; - sigio_unlock(flags); - - if (l_write_sigio_pid != -1) + if (write_sigio_state != INACTIVE) { + sigio_unlock(flags); return; + } else { + write_sigio_state = STARTING; + } + sigio_unlock(flags); - err = os_pipe(l_write_sigio_fds, 1, 1); + err = os_pipe(write_sigio_fds, 1, 1); if(err < 0){ printk("write_sigio_workaround - os_pipe 1 failed, " "err = %d\n", -err); return; } - err = os_pipe(l_sigio_private, 1, 1); + err = os_pipe(sigio_private, 1, 1); if(err < 0){ printk("write_sigio_workaround - os_pipe 2 failed, " "err = %d\n", -err); goto out_close1; } - p = setup_initial_poll(l_sigio_private[1]); + p = setup_initial_poll(sigio_private[1]); if(!p) goto out_close2; - flags = sigio_lock(); - - /* Did we race? Don't try to optimize this, please, it's not so likely - * to happen, and no more than once at the boot. */ - if(write_sigio_pid != -1) - goto out_free; + if (current_poll.poll) + kfree(current_poll.poll); current_poll = ((struct pollfds) { .poll = p, .used = 1, .size = 1 }); - if (write_sigio_irq(l_write_sigio_fds[0])) + if (write_sigio_irq(write_sigio_fds[0])) goto out_clear_poll; - memcpy(write_sigio_fds, l_write_sigio_fds, sizeof(l_write_sigio_fds)); - memcpy(sigio_private, l_sigio_private, sizeof(l_sigio_private)); - write_sigio_pid = run_helper_thread(write_sigio_thread, NULL, CLONE_FILES | CLONE_VM, &stack, 0); if (write_sigio_pid < 0) goto out_clear; + flags = sigio_lock(); + write_sigio_state = ACTIVE; sigio_unlock(flags); return; @@ -312,15 +327,17 @@ out_clear_poll: current_poll = ((struct pollfds) { .poll = NULL, .size = 0, .used = 0 }); -out_free: - sigio_unlock(flags); kfree(p); out_close2: - close(l_sigio_private[0]); - close(l_sigio_private[1]); + close(sigio_private[0]); + close(sigio_private[1]); out_close1: - close(l_write_sigio_fds[0]); - close(l_write_sigio_fds[1]); + close(write_sigio_fds[0]); + close(write_sigio_fds[1]); + + flags = sigio_lock(); + write_sigio_state = BROKEN; + sigio_unlock(flags); } void maybe_sigio_broken(int fd, int read) @@ -357,6 +374,7 @@ static void sigio_cleanup(void) if(write_sigio_pid != -1){ os_kill_process(write_sigio_pid, 1); write_sigio_pid = -1; + write_sigio_state = CLOSED; } } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/