linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it>
To: Andrew Morton <akpm@osdl.org>
Cc: Jeff Dike <jdike@addtoit.com>,
	linux-kernel@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net
Subject: [PATCH 5/9] uml: sigio code - reduce spinlock hold time
Date: Wed, 18 Jan 2006 01:19:36 +0100	[thread overview]
Message-ID: <20060118001935.14622.13619.stgit@zion.home.lan> (raw)
In-Reply-To: <20060117235659.14622.18544.stgit@zion.home.lan>


From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

In a previous patch I shifted an allocation to being atomic.

In this patch, a better but more intrusive solution is implemented, i.e. hold
the lock only when really needing it, especially not over pipe operations, nor
over the culprit allocation.

Additionally, while at it, add a missing kfree in the failure path, and make
sure that if we fail in forking, write_sigio_pid is -1 and not, say, -ENOMEM.

And fix whitespace, at least for things I was touching anyway.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---

 arch/um/kernel/sigio_user.c |   83 ++++++++++++++++++++++++++++++-------------
 1 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/arch/um/kernel/sigio_user.c b/arch/um/kernel/sigio_user.c
index 62e5cfd..55ef415 100644
--- a/arch/um/kernel/sigio_user.c
+++ b/arch/um/kernel/sigio_user.c
@@ -337,70 +337,103 @@ int ignore_sigio_fd(int fd)
 	return(err);
 }
 
-static int setup_initial_poll(int fd)
+static struct pollfd* setup_initial_poll(int fd)
 {
 	struct pollfd *p;
 
-	p = um_kmalloc_atomic(sizeof(struct pollfd));
-	if(p == NULL){
+	p = um_kmalloc(sizeof(struct pollfd));
+	if (p == NULL) {
 		printk("setup_initial_poll : failed to allocate poll\n");
-		return(-1);
+		return NULL;
 	}
 	*p = ((struct pollfd) { .fd  	= fd,
 				.events 	= POLLIN,
 				.revents 	= 0 });
-	current_poll = ((struct pollfds) { .poll 	= p,
-					   .used 	= 1,
-					   .size 	= 1 });
-	return(0);
+	return p;
 }
 
 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;
 
+	/* We call this *tons* of times - and most ones we must just fail. */
 	sigio_lock();
-	if(write_sigio_pid != -1)
-		goto out;
+	l_write_sigio_pid = write_sigio_pid;
+	sigio_unlock();
 
-	err = os_pipe(write_sigio_fds, 1, 1);
+	if (l_write_sigio_pid != -1)
+		return;
+
+	err = os_pipe(l_write_sigio_fds, 1, 1);
 	if(err < 0){
 		printk("write_sigio_workaround - os_pipe 1 failed, "
 		       "err = %d\n", -err);
-		goto out;
+		return;
 	}
-	err = os_pipe(sigio_private, 1, 1);
+	err = os_pipe(l_sigio_private, 1, 1);
 	if(err < 0){
-		printk("write_sigio_workaround - os_pipe 2 failed, "
+		printk("write_sigio_workaround - os_pipe 1 failed, "
 		       "err = %d\n", -err);
 		goto out_close1;
 	}
-	if(setup_initial_poll(sigio_private[1]))
+
+	p = setup_initial_poll(l_sigio_private[1]);
+	if(!p)
 		goto out_close2;
 
-	write_sigio_pid = run_helper_thread(write_sigio_thread, NULL, 
+	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_unlock;
+
+	write_sigio_pid = run_helper_thread(write_sigio_thread, NULL,
 					    CLONE_FILES | CLONE_VM, &stack, 0);
 
-	if(write_sigio_pid < 0) goto out_close2;
+	if (write_sigio_pid < 0)
+		goto out_clear;
 
-	if(write_sigio_irq(write_sigio_fds[0])) 
+	if (write_sigio_irq(l_write_sigio_fds[0])) 
 		goto out_kill;
 
- out:
+	/* Success, finally. */
+	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;
 
  out_kill:
-	os_kill_process(write_sigio_pid, 1);
+	l_write_sigio_pid = write_sigio_pid;
 	write_sigio_pid = -1;
+	sigio_unlock();
+	/* Going to call waitpid, avoid holding the lock. */
+	os_kill_process(l_write_sigio_pid, 1);
+	goto out_free;
+
+ out_clear:
+	write_sigio_pid = -1;
+ out_unlock:
+	sigio_unlock();
+ out_free:
+	kfree(p);
  out_close2:
-	os_close_file(sigio_private[0]);
-	os_close_file(sigio_private[1]);
+	os_close_file(l_sigio_private[0]);
+	os_close_file(l_sigio_private[1]);
  out_close1:
-	os_close_file(write_sigio_fds[0]);
-	os_close_file(write_sigio_fds[1]);
-	sigio_unlock();
+	os_close_file(l_write_sigio_fds[0]);
+	os_close_file(l_write_sigio_fds[1]);
+	return;
 }
 
 int read_sigio_fd(int fd)


  parent reply	other threads:[~2006-01-18  0:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20060117235659.14622.18544.stgit@zion.home.lan>
2006-01-18  0:19 ` [PATCH 1/9] uml: avoid sysfs warning on hot-unplug Paolo 'Blaisorblade' Giarrusso
2006-01-18  2:53   ` Jeff Dike
2006-01-18 11:11     ` Blaisorblade
2006-01-18  0:19 ` [PATCH 2/9] uml: make daemon transport behave properly Paolo 'Blaisorblade' Giarrusso
2006-01-18  0:19 ` [PATCH 3/9] uml: networking - clear transport-specific structure Paolo 'Blaisorblade' Giarrusso
2006-01-18  0:19 ` [PATCH 4/9] uml: fix spinlock recursion and sleep-inside-spinlock in error path Paolo 'Blaisorblade' Giarrusso
2006-01-18  0:52   ` Andrew Morton
2006-01-18 11:44     ` [uml-devel] " Blaisorblade
2006-01-18  0:19 ` Paolo 'Blaisorblade' Giarrusso [this message]
2006-01-18  0:19 ` [PATCH 6/9] uml: avoid malloc to sleep in atomic sections Paolo 'Blaisorblade' Giarrusso
2006-01-18  0:56   ` Andrew Morton
2006-01-18 11:47     ` Blaisorblade
2006-01-18  0:19 ` [PATCH 7/9] uml: arch Kconfig menu cleanups Paolo 'Blaisorblade' Giarrusso
2006-01-18  0:19 ` [PATCH 8/9] uml: allow again to move backing file and to override saved location Paolo 'Blaisorblade' Giarrusso
2006-01-18  0:19 ` [PATCH 9/9] uml ubd code: fix a bit of whitespace Paolo 'Blaisorblade' Giarrusso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060118001935.14622.13619.stgit@zion.home.lan \
    --to=blaisorblade@yahoo.it \
    --cc=akpm@osdl.org \
    --cc=jdike@addtoit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).