uml-patch-2.5.42-1

UML has been updated to 2.5.42 and UML 2.4.19-12. In non-numeric terms, this means the following:
the usual build fixes to keep up with kbuild
the SMP work from UML 2.4.19-12 - it seems reasonably functional
and stable, but there are some unexplained crashes still. Also, the fixes from UML 2.4.19-13 aren't in yet, so an SMP UML will leave children around after exiting (the idle threads for processors 1 ... ncpus - 1). These children will interfere with rebooting, and will also hold down host memory.
various cleanups and bug fixes

The patch is available at

	http://uml-pub.ists.dartmouth.edu/uml/uml-patch-2.5.42-1.bz2

For the other UML mirrors and other downloads, see
http://user-mode-linux.sourceforge.net/dl-sf.html

Other links of interest:

The UML project home page : http://user-mode-linux.sourceforge.net
The UML Community site : http://usermodelinux.org

Jeff

Hello!

For some reason I now need this patch to make bk-current to compile (with 2.5.42-1 patch from you applied, of course).
I do not claim this is correct fix, but at least it works for me ;)

===== drivers/char/random.c 1.24 vs edited =====

@@ -738,7 +738,7 @@
 	__s32		delta, delta2, delta3;
 	int		entropy = 0;
 
-#if defined (__i386__) || defined (__x86_64__)
+#if (defined (__i386__) || defined (__x86_64__)) && !defined (__arch_um__)
 	if (cpu_has_tsc) {
 		__u32 high;
 		rdtsc(time, high);

===== drivers/char/mem.c 1.23 vs edited =====

@@ -132,6 +132,7 @@
 {
 	unsigned long prot = pgprot_val(_prot);
 
+#if !defined(__arch_um__)
 #if defined(__i386__) || defined(__x86_64__)
 	/* On PPro and successors, PCD alone doesn't always mean 
 	    uncached because of interactions with the MTRRs. PCD | PWT

@@ -152,7 +153,7 @@
 	else if (MMU_IS_040 || MMU_IS_060)
 		prot = (prot & _CACHEMASK040) | _PAGE_NOCACHE_S;
 #endif
-
+#endif
 	return __pgprot(prot);
 }
 

@@ -164,7 +165,7 @@
  */
 static inline int noncached_address(unsigned long addr)
 {
-#if defined(__i386__)
+#if defined(__i386__) && !defined(__arch_um__)
 	/* 
 	 * On the PPro and successors, the MTRRs are used to set
 	 * memory types for physical addresses outside main memory, 

I am hitting this same issue. I cannot see what is setting __i386__. A similar hack of undef __i386__ inside the files makes mine work.

-andmike
--
Michael Anderson
[email protected]

That patch is against stock 2.5.42, so I don't make any guarantees about bk-current.

However the __i386__ thing should be taken care of by Makefile-i386 doing

	CFLAGS += -U__i386__

I might have messed up the patch, I'll check and fix it if so.

Jeff

Hello!

I am in no way inplying that you are making any guarantees about your patches to work with something but the kernels they are released for.
On the other hand I thought you might find it useful if I report to you problems with more modern kernels that I encounter so that when you will update UML to never kernel you do not need to hit all the problems by yourself.

Yes, it seems to be the case.

CFLAGS is defined first in arch/um/Makefile and only then you do include Makefile-{SUBARCH}

Moving 'include $(ARCH_DIR)/Makefile-$(SUBARCH)' in front of CFLAGS helped.

Bye,
Oleg

I was seeing the same failure on 2.5.42.

This fixed the __i386__ issue, but messed up the header symlinks. I moved CFLAGS after the includes and this appears to have fixed both problems, but makefile magic is not my thing so YMMV.

-andmike
--
Michael Anderson
[email protected]

I didn't think you were. I just wanted to make sure that was clear.

In any case, that's not the problem. I botched the patch. I'll be releasing a fixed patch shortly.

Jeff

Hello!

Reviewing your patch I spotted some locking inconsistencies.
Also Nikita Danilov have spotted a double unlock in irq_user.c:reactivate_fd(), you forgot to do return before "out" label.
Below is quotes from the patch, and at the very end of the message there is a my proposed patch to fix all uncovered problems, patch was tested as in "compiles and runs ok for me".

Also
arch/um/drivers/ubd_kern.c:
@@ -892,8 +948,11 @@

        n = minor(rdev) >> UBD_SHIFT;
        dev = &ubd_dev[n];
+
+       err = 0;
+       spin_lock(&ubd_lock);
        if(dev->is_dir)

- return(0);
+ goto out;

        err = ubd_file_size(dev, &size);
        if (!err) {
@@ -902,7 +961,8 @@
                        set_capacity(fake_gendisk[n], size / 512);
                dev->size = size;
        }
-
+       spin_unlock(&ubd_lock);
+ out:
        return err;

}

Here you forget to unlock ubd_lock if dev->is_dir is true.

arch/um/kernel/irq_user.c
+ /* Actually, it only looks like it can be called from interrupt

+        * context.  The culprit is reactivate_fd, which calls
+        * maybe_sigio_broken, which calls write_sigio_workaround,
+        * which calls activate_fd.  However, write_sigio_workaround should
+        * only be called once, at boot time.  That would make it clear that
+        * this is called only from process context, and can be locked with
+        * a semaphore.
+        */
+       flags = irq_lock();
+       for(irq_fd = active_fds; irq_fd != NULL; irq_fd = irq_fd->next){
+               if((irq_fd->fd == fd) && (irq_fd->type == type)){
+                       printk("Registering fd %d twice\n", fd);
+                       printk("Irqs : %d, %d\n", irq_fd->irq, irq);
+                       printk("Ids : 0x%x, 0x%x\n", irq_fd->id, dev_id);
+                       goto out_free;

+ }
+ }
...
+ out_unlock:
+ irq_unlock(flags);
+ out_free:

        kfree(new_fd);
+ out:
        return(err);

Here you forgot to do irq_unlock()

Also there are number "32" is hardcoded into arch/um/kernel/trap_user.c in some arrays, taht you probably actually want to make dependent on CONFIG_NR_CPUS

Bye,
Oleg
===== arch/um/drivers/ubd_kern.c 1.9 vs edited =====

@@ -961,8 +961,8 @@
 			set_capacity(fake_gendisk[n], size / 512);
 		dev->size = size;
 	}
-	spin_unlock(&ubd_lock);
  out:
+	spin_unlock(&ubd_lock);
 	return err;
 }
 

===== arch/um/kernel/irq_user.c 1.3 vs edited =====

@@ -157,7 +157,7 @@
 			printk("Registering fd %d twice\n", fd);
 			printk("Irqs : %d, %d\n", irq_fd->irq, irq);
 			printk("Ids : 0x%x, 0x%x\n", irq_fd->id, dev_id);
-			goto out_free;
+			goto out_unlock;
 		}
 	}
 

@@ -209,7 +209,6 @@
 
  out_unlock:
 	irq_unlock(flags);
- out_free:
 	kfree(new_fd);
  out:
 	return(err);

@@ -344,6 +343,7 @@
 	 * section.
 	 */
 	maybe_sigio_broken(fd, irq->type);
+	return;
  out:
 	irq_unlock(flags);
 }

Correct on all counts. Nice spotting.

The problem is that trap_user.c is a userspace file and has no access to config.h. The hardcoded 32 is nasty and needs fixing, but I haven't decided how to do that yet.

Jeff