[PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Paolo 'Blaisorblade' Giarrusso]

From: Jeff Dike <jdike@addtoit.com>, Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

Define a release method for the ubd and network driver so that sysfs doesn't
complain when one is removed via:

host $ uml_mconsole <umid> remove <dev>

Done by Jeff around January for ubd only, later lost, then restored in his tree
- however I'm merging it now since there's no reason to leave this here.

We don't need to do any cleanup in the new added method, because when hot-unplug
is done by uml_mconsole we already handle cleanup in mconsole infrastructure,
i.e.  mc_device->remove (net_remove/ubd_remove), which is also the calling
method.

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

 arch/um/drivers/net_kern.c |    6 ++++++
 arch/um/drivers/ubd_kern.c |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
index fb1f9fb..5b8c64e 100644
--- a/arch/um/drivers/net_kern.c
+++ b/arch/um/drivers/net_kern.c
@@ -276,6 +276,11 @@ static struct platform_driver uml_net_dr
 };
 static int driver_registered;
 
+/* Sysfs requires a ->release when mconsole hot-unplug is done */
+static void dummy_device_release(struct device *dev)
+{
+}
+
 static int eth_configure(int n, void *init, char *mac,
 			 struct transport *transport)
 {
@@ -324,6 +329,7 @@ static int eth_configure(int n, void *in
 	}
 	device->pdev.id = n;
 	device->pdev.name = DRIVER_NAME;
+	device->pdev.dev.release = dummy_device_release;
 	platform_device_register(&device->pdev);
 	SET_NETDEV_DEV(dev,&device->pdev.dev);
 
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 7696f8d..49dda56 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -617,6 +617,11 @@ static int ubd_open_dev(struct ubd *dev)
 	return(err);
 }
 
+/* Sysfs requires a ->release when mconsole hot-unplug is done */
+static void dummy_device_release(struct device *dev)
+{
+}
+
 static int ubd_new_disk(int major, u64 size, int unit,
 			struct gendisk **disk_out)
 			
@@ -652,6 +657,7 @@ static int ubd_new_disk(int major, u64 s
 	if (major == MAJOR_NR) {
 		ubd_dev[unit].pdev.id   = unit;
 		ubd_dev[unit].pdev.name = DRIVER_NAME;
+		ubd_dev[unit].pdev.dev.release = dummy_device_release;
 		platform_device_register(&ubd_dev[unit].pdev);
 		disk->driverfs_dev = &ubd_dev[unit].pdev.dev;
 	}

Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Jeff Dike]

On Wed, Jan 18, 2006 at 01:19:21AM +0100, Paolo 'Blaisorblade' Giarrusso wrote:
> Done by Jeff around January for ubd only, later lost, then restored in his tree
> - however I'm merging it now since there's no reason to leave this here.

The original was dinged by hch for covering over a problem without really
fixing it.

We need to think about this one some more.

				Jeff

Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Blaisorblade]

On Wednesday 18 January 2006 03:53, Jeff Dike wrote:
> On Wed, Jan 18, 2006 at 01:19:21AM +0100, Paolo 'Blaisorblade' Giarrusso 
wrote:
> > Done by Jeff around January for ubd only, later lost, then restored in
> > his tree - however I'm merging it now since there's no reason to leave
> > this here.
>
> The original was dinged by hch for covering over a problem without really
> fixing it.
>
> We need to think about this one some more.

About this, have you reported upstream the "crash with remove ubd2 and ubd3" 
thing caused by wrong refcounts? Greg and friends tend to fix various drivers 
all around, I guess it's the right policy to ask them to fix this too.

-- 
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! Messenger with Voice: chiama da PC a telefono a tariffe esclusive 
http://it.messenger.yahoo.com

[PATCH 2/9] uml: make daemon transport behave properly

[Paolo 'Blaisorblade' Giarrusso]

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

Avoid uninitialized data in the daemon_data structure. I used this transport
before doing proper setup before-hand, and I got some very nice SLAB corruption
due to freeing crap pointers. So just make sure to clear everything when
appropriate.

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

 arch/um/drivers/daemon_kern.c |    4 ++++
 arch/um/drivers/daemon_user.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/um/drivers/daemon_kern.c b/arch/um/drivers/daemon_kern.c
index 30d285b..507e3cb 100644
--- a/arch/um/drivers/daemon_kern.c
+++ b/arch/um/drivers/daemon_kern.c
@@ -31,6 +31,10 @@ void daemon_init(struct net_device *dev,
 	dpri->fd = -1;
 	dpri->control = -1;
 	dpri->dev = dev;
+	/* We will free this pointer. If it contains crap we're burned. */
+	dpri->ctl_addr = NULL;
+	dpri->data_addr = NULL;
+	dpri->local_addr = NULL;
 
 	printk("daemon backend (uml_switch version %d) - %s:%s", 
 	       SWITCH_VERSION, dpri->sock_type, dpri->ctl_sock);
diff --git a/arch/um/drivers/daemon_user.c b/arch/um/drivers/daemon_user.c
index 1bb085b..c944265 100644
--- a/arch/um/drivers/daemon_user.c
+++ b/arch/um/drivers/daemon_user.c
@@ -158,10 +158,16 @@ static void daemon_remove(void *data)
 	struct daemon_data *pri = data;
 
 	os_close_file(pri->fd);
+	pri->fd = -1;
 	os_close_file(pri->control);
+	pri->control = -1;
+
 	kfree(pri->data_addr);
+	pri->data_addr = NULL;
 	kfree(pri->ctl_addr);
+	pri->ctl_addr = NULL;
 	kfree(pri->local_addr);
+	pri->local_addr = NULL;
 }
 
 int daemon_user_write(int fd, void *buf, int len, struct daemon_data *pri)

[PATCH 3/9] uml: networking - clear transport-specific structure

[Paolo 'Blaisorblade' Giarrusso]

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

Pre-clear transport-specific private structure before passing it down.

In fact, I just got a slab corruption and kernel panic on exit because kfree()
was called on a pointer which probably was never allocated, BUT hadn't been set
to NULL by the driver.

As the code is full of such errors, I've decided for now to go the safe way
(we're talking about drivers), and to do the simple thing. I'm also starting to
fix drivers, and already sent a patch for the daemon transport.

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

 arch/um/drivers/net_kern.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
index 5b8c64e..98350bb 100644
--- a/arch/um/drivers/net_kern.c
+++ b/arch/um/drivers/net_kern.c
@@ -322,6 +322,11 @@ static int eth_configure(int n, void *in
 		return 1;
 	}
 
+	lp = dev->priv;
+	/* This points to the transport private data. It's still clear, but we
+	 * must memset it to 0 *now*. Let's help the drivers. */
+	memset(lp, 0, size);
+
 	/* sysfs register */
 	if (!driver_registered) {
 		platform_driver_register(&uml_net_driver);
@@ -364,7 +369,6 @@ static int eth_configure(int n, void *in
 		free_netdev(dev);
 		return 1;
 	}
-	lp = dev->priv;
 
 	/* lp.user is the first four bytes of the transport data, which
 	 * has already been initialized.  This structure assignment will

[PATCH 4/9] uml: fix spinlock recursion and sleep-inside-spinlock in error path

[Paolo 'Blaisorblade' Giarrusso]

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

In this error path, when the interface has had a problem, we call dev_close(),
which is disallowed for two reasons:

*) takes again the UML internal spinlock, inside the ->stop method of this
   device
*) can be called in process context only, while we're in interrupt context.

I've also thought that calling dev_close() may be a wrong policy to follow, but
it's not up to me to decide that.

However, we may end up with multiple dev_close() queued on the same device.
But the initial test for (dev->flags & IFF_UP) makes this harmless, though -
and dev_close() is supposed to care about races with itself. So there's no harm
in delaying the shutdown, IMHO.

Something to mark the interface as "going to shutdown" would be appreciated, but
dev_deactivate has the same problems as dev_close(), so we can't use it either.

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

 arch/um/drivers/net_kern.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
index 98350bb..178f68b 100644
--- a/arch/um/drivers/net_kern.c
+++ b/arch/um/drivers/net_kern.c
@@ -68,6 +68,11 @@ static int uml_net_rx(struct net_device 
 	return pkt_len;
 }
 
+static void uml_dev_close(void* dev)
+{
+	dev_close( (struct net_device *) dev);
+}
+
 irqreturn_t uml_net_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
 	struct net_device *dev = dev_id;
@@ -80,15 +85,21 @@ irqreturn_t uml_net_interrupt(int irq, v
 	spin_lock(&lp->lock);
 	while((err = uml_net_rx(dev)) > 0) ;
 	if(err < 0) {
+		DECLARE_WORK(close_work, uml_dev_close, dev);
 		printk(KERN_ERR 
 		       "Device '%s' read returned %d, shutting it down\n", 
 		       dev->name, err);
-		dev_close(dev);
+		/* dev_close can't be called in interrupt context, and takes
+		 * again lp->lock.
+		 * And dev_close() can be safely called multiple times on the
+		 * same device, since it tests for (dev->flags & IFF_UP). So
+		 * there's no harm in delaying the device shutdown. */
+		schedule_work(&close_work);
 		goto out;
 	}
 	reactivate_fd(lp->fd, UM_ETH_IRQ);
 
- out:
+out:
 	spin_unlock(&lp->lock);
 	return(IRQ_HANDLED);
 }

Re: [PATCH 4/9] uml: fix spinlock recursion and sleep-inside-spinlock in error path

[Andrew Morton]

"Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it> wrote:
>
> 
> From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> 
> In this error path, when the interface has had a problem, we call dev_close(),
> which is disallowed for two reasons:
> 
> *) takes again the UML internal spinlock, inside the ->stop method of this
>    device
> *) can be called in process context only, while we're in interrupt context.
> 
> I've also thought that calling dev_close() may be a wrong policy to follow, but
> it's not up to me to decide that.
> 
> However, we may end up with multiple dev_close() queued on the same device.
> But the initial test for (dev->flags & IFF_UP) makes this harmless, though -
> and dev_close() is supposed to care about races with itself. So there's no harm
> in delaying the shutdown, IMHO.
> 
> Something to mark the interface as "going to shutdown" would be appreciated, but
> dev_deactivate has the same problems as dev_close(), so we can't use it either.
> 
> ...
> +		/* dev_close can't be called in interrupt context, and takes
> +		 * again lp->lock.
> +		 * And dev_close() can be safely called multiple times on the
> +		 * same device, since it tests for (dev->flags & IFF_UP). So
> +		 * there's no harm in delaying the device shutdown. */
> +		schedule_work(&close_work);
>  		goto out;
>  	}

This callback can be pending for an arbitrary amount of time.  I'd have
expected to see a flush_sceduled_work() somewhere in the driver to force
all such pending work to complete before we destroy things which that
callback wil be using.

Re: [uml-devel] Re: [PATCH 4/9] uml: fix spinlock recursion and sleep-inside-spinlock in error path

[Blaisorblade]

On Wednesday 18 January 2006 01:52, Andrew Morton wrote:
> "Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it> wrote:
> > From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> >
> > In this error path, when the interface has had a problem, we call
> > dev_close(), which is disallowed for two reasons:

> > *) takes again the UML internal spinlock, inside the ->stop method of
> > this device
> > *) can be called in process context only, while we're in interrupt
> > context.

> > I've also thought that calling dev_close() may be a wrong policy to
> > follow, but it's not up to me to decide that.

> > However, we may end up with multiple dev_close() queued on the same
> > device. But the initial test for (dev->flags & IFF_UP) makes this
> > harmless, though - and dev_close() is supposed to care about races with
> > itself. So there's no harm in delaying the shutdown, IMHO.

> > Something to mark the interface as "going to shutdown" would be
> > appreciated, but dev_deactivate has the same problems as dev_close(), so
> > we can't use it either.

> > ...
> > +		/* dev_close can't be called in interrupt context, and takes
> > +		 * again lp->lock.
> > +		 * And dev_close() can be safely called multiple times on the
> > +		 * same device, since it tests for (dev->flags & IFF_UP). So
> > +		 * there's no harm in delaying the device shutdown. */
> > +		schedule_work(&close_work);
> >  		goto out;
> >  	}

> This callback can be pending for an arbitrary amount of time.  I'd have
> expected to see a flush_sceduled_work() somewhere in the driver to force
> all such pending work to complete before we destroy things which that
> callback wil be using.

Thanks for the tip (it's good I got schedule_work right, it wasn't hard but 
1st time I use it), however the device is not later freed, it seems, in this 
context... even because it's an unlikely event (I reproduced it with a 
damn-bad configuration).

Not going to cause seeable leaks anyway, but yes, to correct.

However, network devices are created from the host only, so only the host 
should free them (as done by net_remove on "hot-unplug" request).

Maybe that's to fix too, however, but in a separate patch; Jeff, what about 
this?
-- 
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! Messenger with Voice: chiama da PC a telefono a tariffe esclusive 
http://it.messenger.yahoo.com

[PATCH 5/9] uml: sigio code - reduce spinlock hold time

[Paolo 'Blaisorblade' Giarrusso]

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)

[PATCH 6/9] uml: avoid malloc to sleep in atomic sections

[Paolo 'Blaisorblade' Giarrusso]

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

Ugly trick to help make malloc not sleeping - we can't do anything else. But
this is not yet optimal, since spinlock don't trigger in_atomic() when
preemption is disabled.

Also, even if ugly, this was already used in one place, and was even more bogus.
Fix it.

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

 arch/um/include/kern_util.h   |    4 +++-
 arch/um/include/user.h        |    1 +
 arch/um/kernel/process_kern.c |   21 +++++++++++++--------
 arch/um/os-Linux/helper.c     |    4 ++--
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/arch/um/include/kern_util.h b/arch/um/include/kern_util.h
index 8f4e46d..c649108 100644
--- a/arch/um/include/kern_util.h
+++ b/arch/um/include/kern_util.h
@@ -120,8 +120,10 @@ extern void machine_halt(void);
 extern int is_syscall(unsigned long addr);
 extern void arch_switch(void);
 extern void free_irq(unsigned int, void *);
-extern int um_in_interrupt(void);
 extern int cpu(void);
+
+/* Are we disallowed to sleep? Used to choose between GFP_KERNEL and GFP_ATOMIC. */
+extern int __cant_sleep(void);
 extern void segv_handler(int sig, union uml_pt_regs *regs);
 extern void sigio_handler(int sig, union uml_pt_regs *regs);
 
diff --git a/arch/um/include/user.h b/arch/um/include/user.h
index 0f865ef..91b0ac4 100644
--- a/arch/um/include/user.h
+++ b/arch/um/include/user.h
@@ -18,6 +18,7 @@ extern int open_gdb_chan(void);
 extern unsigned long strlcpy(char *, const char *, unsigned long);
 extern unsigned long strlcat(char *, const char *, unsigned long);
 extern void *um_vmalloc(int size);
+extern void *um_vmalloc_atomic(int size);
 extern void vfree(void *ptr);
 
 #endif
diff --git a/arch/um/kernel/process_kern.c b/arch/um/kernel/process_kern.c
index 7f13b85..d852c55 100644
--- a/arch/um/kernel/process_kern.c
+++ b/arch/um/kernel/process_kern.c
@@ -288,17 +288,27 @@ EXPORT_SYMBOL(disable_hlt);
 
 void *um_kmalloc(int size)
 {
-	return(kmalloc(size, GFP_KERNEL));
+	return kmalloc(size, GFP_KERNEL);
 }
 
 void *um_kmalloc_atomic(int size)
 {
-	return(kmalloc(size, GFP_ATOMIC));
+	return kmalloc(size, GFP_ATOMIC);
 }
 
 void *um_vmalloc(int size)
 {
-	return(vmalloc(size));
+	return vmalloc(size);
+}
+
+void *um_vmalloc_atomic(int size)
+{
+	return __vmalloc(size, GFP_ATOMIC | __GFP_HIGHMEM, PAGE_KERNEL);
+}
+
+int __cant_sleep(void) {
+	return in_atomic() || irqs_disabled() || in_interrupt();
+	/* Is in_interrupt() really needed? */
 }
 
 unsigned long get_fault_addr(void)
@@ -370,11 +380,6 @@ int smp_sigio_handler(void)
 	return(0);
 }
 
-int um_in_interrupt(void)
-{
-	return(in_interrupt());
-}
-
 int cpu(void)
 {
 	return(current_thread->cpu);
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index 36cc847..6490a4f 100644
--- a/arch/um/os-Linux/helper.c
+++ b/arch/um/os-Linux/helper.c
@@ -60,7 +60,7 @@ int run_helper(void (*pre_exec)(void *),
 
 	if((stack_out != NULL) && (*stack_out != 0))
 		stack = *stack_out;
-	else stack = alloc_stack(0, um_in_interrupt());
+	else stack = alloc_stack(0, __cant_sleep());
 	if(stack == 0)
 		return(-ENOMEM);
 
@@ -124,7 +124,7 @@ int run_helper_thread(int (*proc)(void *
 	unsigned long stack, sp;
 	int pid, status, err;
 
-	stack = alloc_stack(stack_order, um_in_interrupt());
+	stack = alloc_stack(stack_order, __cant_sleep());
 	if(stack == 0) return(-ENOMEM);
 
 	sp = stack + (page_size() << stack_order) - sizeof(void *);

Re: [PATCH 6/9] uml: avoid malloc to sleep in atomic sections

[Andrew Morton]

"Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it> wrote:
>
> +int __cant_sleep(void) {
> +	return in_atomic() || irqs_disabled() || in_interrupt();

aww, man, this is awful.  Code is supposed to know what context it's
running in, not go fishing about in core internals trying to fix up its own
confusion.

> +	/* Is in_interrupt() really needed? */
>  }

Yes, it is.  in_atomic() is a no-op on !PREEMPT and local irq's can be
enabled in soft- or hard- interrupt context, so irqs_disabled() will return
0.

Re: [PATCH 6/9] uml: avoid malloc to sleep in atomic sections

[Blaisorblade]

On Wednesday 18 January 2006 01:56, Andrew Morton wrote:
> "Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it> wrote:
> > +int __cant_sleep(void) {
> > +	return in_atomic() || irqs_disabled() || in_interrupt();
>
> aww, man, this is awful.  Code is supposed to know what context it's
> running in, not go fishing about in core internals trying to fix up its own
> confusion.

Yes, I know.

But is libc supposed too? That's our problem. This code is only used for calls 
to malloc() (not kmalloc()) after the kernel has booted.

There are a few calls to malloc in our code, ok, and those should be fixed 
too.

But the main problem are glibc ones (which are called from all kinds of places 
- we once got a crash because opendir() was calling malloc with big sizes - 
we solved that by optionally using vmalloc()).
-- 
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! Messenger with Voice: chiama da PC a telefono a tariffe esclusive 
http://it.messenger.yahoo.com

[PATCH 7/9] uml: arch Kconfig menu cleanups

[Paolo 'Blaisorblade' Giarrusso]

*) mark as "EXPERIMENTAL" various items that either aren't very stable or that
are actively crashing the setup of users which don't really need them (i.e.
HIGHMEM and 3-level pagetables on x86 - nobody needs either, everybody reports
"I'm using it and getting trouble").

*) move net/Kconfig near to the rest of network configurations, and
drivers/block/Kconfig near "Block layer" submenu.

*) it's useless and doesn't work well to force NETDEVICES on and to disable the
prompt like it's done. Better remove the attempt, and change that to a simple
"default y if UML".

*) drop the warning about "report problems about HPPFS" - it's redundant
anyway, as that's the usual procedure, and HPPFS users are especially technical
(i.e. they know reporting bugs is _good_).

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

 arch/um/Kconfig      |   27 ++++++++++++++-------------
 arch/um/Kconfig.i386 |    6 +++++-
 drivers/net/Kconfig  |    1 +
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 8ff3bcb..5982fe2 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -143,6 +143,7 @@ config HOSTFS
 
 config HPPFS
 	tristate "HoneyPot ProcFS (EXPERIMENTAL)"
+	depends on EXPERIMENTAL
 	help
 	hppfs (HoneyPot ProcFS) is a filesystem which allows UML /proc
 	entries to be overridden, removed, or fabricated from the host.
@@ -155,10 +156,6 @@ config HPPFS
 	You only need this if you are setting up a UML honeypot.  Otherwise,
 	it is safe to say 'N' here.
 
-	If you are actively using it, please report any problems, since it's
-	getting fixed. In this moment, it is experimental on 2.6 (it works on
-	2.4).
-
 config MCONSOLE
 	bool "Management console"
 	default y
@@ -243,8 +240,16 @@ config NEST_LEVEL
         Only change this if you are running nested UMLs.
 
 config HIGHMEM
-	bool "Highmem support"
-	depends on !64BIT
+	bool "Highmem support (EXPERIMENTAL)"
+	depends on !64BIT && EXPERIMENTAL
+	default n
+	help
+	This was used to allow UML to run with big amounts of memory.
+	Currently it is unstable, so if unsure say N.
+
+	To use big amounts of memory, it is recommended to disable TT mode (i.e.
+	CONFIG_MODE_TT) and enable static linking (i.e. CONFIG_STATIC_LINK) -
+	this should allow the guest to use up to 2.75G of memory.
 
 config KERNEL_STACK_ORDER
 	int "Kernel stack size order"
@@ -269,17 +274,13 @@ endmenu
 
 source "init/Kconfig"
 
-source "net/Kconfig"
-
-source "drivers/base/Kconfig"
+source "drivers/block/Kconfig"
 
 source "arch/um/Kconfig.char"
 
-source "drivers/block/Kconfig"
+source "drivers/base/Kconfig"
 
-config NETDEVICES
-	bool
-	default NET
+source "net/Kconfig"
 
 source "arch/um/Kconfig.net"
 
diff --git a/arch/um/Kconfig.i386 b/arch/um/Kconfig.i386
index c71b39a..ef79ed2 100644
--- a/arch/um/Kconfig.i386
+++ b/arch/um/Kconfig.i386
@@ -22,13 +22,17 @@ config TOP_ADDR
  	default 0x80000000 if HOST_2G_2G
 
 config 3_LEVEL_PGTABLES
-	bool "Three-level pagetables"
+	bool "Three-level pagetables (EXPERIMENTAL)"
 	default n
+	depends on EXPERIMENTAL
 	help
 	Three-level pagetables will let UML have more than 4G of physical
 	memory.  All the memory that can't be mapped directly will be treated
 	as high memory.
 
+	However, this it experimental on 32-bit architectures, so if unsure say
+	N (on x86-64 it's automatically enabled, instead, as it's safe there).
+
 config STUB_CODE
 	hex
 	default 0xbfffe000
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 1421941..c0f9351 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -7,6 +7,7 @@ menu "Network device support"
 
 config NETDEVICES
 	depends on NET
+	default y if UML
 	bool "Network device support"
 	---help---
 	  You can say N here if you don't intend to connect your Linux box to

[PATCH 8/9] uml: allow again to move backing file and to override saved location

[Paolo 'Blaisorblade' Giarrusso]

When the user specifies both a COW file and its backing file, if the previous
backing file is not found, currently UML tries again to use it and fails.

This can be corrected by changing same_backing_files() return value in that
case, so that the caller will try to change the COW file to point to the new
location, as already done in other cases.

Additionally, given the change in the meaning of the func, change its name,
invert its return value, so all values are inverted except when
stat(from_cow,&buf2) fails. And add some comments and two minor bugfixes -
remove a fd leak (return err rather than goto out) and a repeated check.

Tested well.

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

 arch/um/drivers/ubd_kern.c |   28 +++++++++++++++-------------
 1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 49dda56..e498f34 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1109,31 +1109,33 @@ static int ubd_ioctl(struct inode * inod
 	return(-EINVAL);
 }
 
-static int same_backing_files(char *from_cmdline, char *from_cow, char *cow)
+static int path_requires_switch(char *from_cmdline, char *from_cow, char *cow)
 {
 	struct uml_stat buf1, buf2;
 	int err;
 
-	if(from_cmdline == NULL) return(1);
-	if(!strcmp(from_cmdline, from_cow)) return(1);
+	if(from_cmdline == NULL)
+		return 0;
+	if(!strcmp(from_cmdline, from_cow))
+		return 0;
 
 	err = os_stat_file(from_cmdline, &buf1);
 	if(err < 0){
 		printk("Couldn't stat '%s', err = %d\n", from_cmdline, -err);
-		return(1);
+		return 0;
 	}
 	err = os_stat_file(from_cow, &buf2);
 	if(err < 0){
 		printk("Couldn't stat '%s', err = %d\n", from_cow, -err);
-		return(1);
+		return 1;
 	}
 	if((buf1.ust_dev == buf2.ust_dev) && (buf1.ust_ino == buf2.ust_ino))
-		return(1);
+		return 0;
 
 	printk("Backing file mismatch - \"%s\" requested,\n"
 	       "\"%s\" specified in COW header of \"%s\"\n",
 	       from_cmdline, from_cow, cow);
-	return(0);
+	return 1;
 }
 
 static int backing_file_mismatch(char *file, __u64 size, time_t mtime)
@@ -1195,7 +1197,7 @@ int open_ubd_file(char *file, struct ope
 	unsigned long long size;
 	__u32 version, align;
 	char *backing_file;
-	int fd, err, sectorsize, same, mode = 0644;
+	int fd, err, sectorsize, asked_switch, mode = 0644;
 
 	fd = os_open_file(file, *openflags, mode);
 	if(fd < 0){
@@ -1215,6 +1217,7 @@ int open_ubd_file(char *file, struct ope
 		goto out_close;
 	}
 
+	/* Succesful return case! */
 	if(backing_file_out == NULL) return(fd);
 
 	err = read_cow_header(file_reader, &fd, &version, &backing_file, &mtime,
@@ -1226,17 +1229,16 @@ int open_ubd_file(char *file, struct ope
 	}
 	if(err) return(fd);
 
-	if(backing_file_out == NULL) return(fd);
-
-	same = same_backing_files(*backing_file_out, backing_file, file);
+	asked_switch = path_requires_switch(*backing_file_out, backing_file, file);
 
-	if(!same && !backing_file_mismatch(*backing_file_out, size, mtime)){
+	/* Allow switching only if no mismatch. */
+	if (asked_switch && !backing_file_mismatch(*backing_file_out, size, mtime)) {
 		printk("Switching backing file to '%s'\n", *backing_file_out);
 		err = write_cow_header(file, fd, *backing_file_out,
 				       sectorsize, align, &size);
 		if(err){
 			printk("Switch failed, errno = %d\n", -err);
-			return(err);
+			goto out_close;
 		}
 	}
 	else {

[PATCH 9/9] uml ubd code: fix a bit of whitespace

[Paolo 'Blaisorblade' Giarrusso]

Correct a bit of whitespace problems while working here.

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

 arch/um/drivers/ubd_kern.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index e498f34..f93af66 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1200,15 +1200,16 @@ int open_ubd_file(char *file, struct ope
 	int fd, err, sectorsize, asked_switch, mode = 0644;
 
 	fd = os_open_file(file, *openflags, mode);
-	if(fd < 0){
-		if((fd == -ENOENT) && (create_cow_out != NULL))
+	if (fd < 0) {
+		if ((fd == -ENOENT) && (create_cow_out != NULL))
 			*create_cow_out = 1;
-                if(!openflags->w ||
-                   ((fd != -EROFS) && (fd != -EACCES))) return(fd);
+                if (!openflags->w ||
+                   ((fd != -EROFS) && (fd != -EACCES)))
+			return fd;
 		openflags->w = 0;
 		fd = os_open_file(file, *openflags, mode);
-		if(fd < 0)
-			return(fd);
+		if (fd < 0)
+			return fd;
         }
 
 	err = os_lock_file(fd, openflags->w);
@@ -1218,7 +1219,8 @@ int open_ubd_file(char *file, struct ope
 	}
 
 	/* Succesful return case! */
-	if(backing_file_out == NULL) return(fd);
+	if(backing_file_out == NULL)
+		return(fd);
 
 	err = read_cow_header(file_reader, &fd, &version, &backing_file, &mtime,
 			      &size, &sectorsize, &align, bitmap_offset_out);
@@ -1227,7 +1229,8 @@ int open_ubd_file(char *file, struct ope
 		       "errno = %d\n", file, -err);
 		goto out_close;
 	}
-	if(err) return(fd);
+	if(err)
+		return(fd);
 
 	asked_switch = path_requires_switch(*backing_file_out, backing_file, file);
 
@@ -1236,24 +1239,24 @@ int open_ubd_file(char *file, struct ope
 		printk("Switching backing file to '%s'\n", *backing_file_out);
 		err = write_cow_header(file, fd, *backing_file_out,
 				       sectorsize, align, &size);
-		if(err){
+		if (err) {
 			printk("Switch failed, errno = %d\n", -err);
 			goto out_close;
 		}
-	}
-	else {
+	} else {
 		*backing_file_out = backing_file;
 		err = backing_file_mismatch(*backing_file_out, size, mtime);
-		if(err) goto out_close;
+		if (err)
+			goto out_close;
 	}
 
 	cow_sizes(version, size, sectorsize, align, *bitmap_offset_out,
 		  bitmap_len_out, data_offset_out);
 
-        return(fd);
+        return fd;
  out_close:
 	os_close_file(fd);
-	return(err);
+	return err;
 }
 
 int create_cow_file(char *cow_file, char *backing_file, struct openflags flags,

Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Greg KH]

> From: Jeff Dike <jdike@addtoit.com>, Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> 
> Define a release method for the ubd and network driver so that sysfs doesn't
> complain when one is removed via:

What?  No.  The kernel is complaining for a reason, don't try to
out-smart it.

> 
> host $ uml_mconsole <umid> remove <dev>
> 
> Done by Jeff around January for ubd only, later lost, then restored in his tree
> - however I'm merging it now since there's no reason to leave this here.
> 
> We don't need to do any cleanup in the new added method, because when hot-unplug
> is done by uml_mconsole we already handle cleanup in mconsole infrastructure,
> i.e.  mc_device->remove (net_remove/ubd_remove), which is also the calling
> method.

Huh?  You have 2 different release functions for the same object?  And
how do you know which one is correct?  That does not sound right at all.

Please fix this correctly.

thanks,

greg k-h