uml-patch-2.5.42-1

uml-patch-2.5.42-1

[Jeff Dike]

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

Re: uml-patch-2.5.42-1

[Oleg Drokin]

Hello!

On Mon, Oct 14, 2002 at 07:58:28PM -0500, Jeff Dike wrote:
> UML has been updated to 2.5.42 and UML 2.4.19-12.  In non-numeric terms,

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 =====
--- 1.24/drivers/char/random.c	Mon Oct  7 18:38:26 2002
+++ edited/drivers/char/random.c	Tue Oct 15 10:20:50 2002
@@ -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 =====
--- 1.23/drivers/char/mem.c	Mon Aug  5 23:05:22 2002
+++ edited/drivers/char/mem.c	Tue Oct 15 10:18:31 2002
@@ -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, 

Re: [uml-devel] Re: uml-patch-2.5.42-1

[Mike Anderson]

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.

Oleg Drokin [green@namesys.com] wrote:
> Hello!
> 
> On Mon, Oct 14, 2002 at 07:58:28PM -0500, Jeff Dike wrote:
> > UML has been updated to 2.5.42 and UML 2.4.19-12.  In non-numeric terms,
> 
> 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 =====
> --- 1.24/drivers/char/random.c	Mon Oct  7 18:38:26 2002
> +++ edited/drivers/char/random.c	Tue Oct 15 10:20:50 2002
> @@ -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 =====
> --- 1.23/drivers/char/mem.c	Mon Aug  5 23:05:22 2002
> +++ edited/drivers/char/mem.c	Tue Oct 15 10:18:31 2002
> @@ -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, 
> 
> 
> -------------------------------------------------------
> This sf.net email is sponsored by:ThinkGeek
> Welcome to geek heaven.
> http://thinkgeek.com/sf
> _______________________________________________
> User-mode-linux-devel mailing list
> User-mode-linux-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: uml-patch-2.5.42-1

[Oleg Drokin]

Hello!

On Mon, Oct 14, 2002 at 07:58:28PM -0500, Jeff Dike wrote:
> 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

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 =====
--- 1.9/arch/um/drivers/ubd_kern.c	Tue Oct 15 10:44:19 2002
+++ edited/arch/um/drivers/ubd_kern.c	Tue Oct 15 15:18:43 2002
@@ -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 =====
--- 1.3/arch/um/kernel/irq_user.c	Tue Oct 15 10:44:20 2002
+++ edited/arch/um/kernel/irq_user.c	Tue Oct 15 15:21:16 2002
@@ -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);
 }

Re: uml-patch-2.5.42-1

[Oleg Drokin]

Hello!

On Tue, Oct 15, 2002 at 08:52:17AM -0500, Jeff Dike wrote:

> > For some reason I now need this patch to make bk-current to compile 
> That patch is against stock 2.5.42, so I don't make any guarantees about
> bk-current.

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.

> 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.

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

Re: uml-patch-2.5.42-1

[Jeff Dike]

green@namesys.com said:
> For some reason I now need this patch to make bk-current to compile 

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

Re: [uml-devel] Re: uml-patch-2.5.42-1

[Mike Anderson]

Oleg Drokin [green@namesys.com] wrote:
> Hello!
> 
> On Tue, Oct 15, 2002 at 08:52:17AM -0500, Jeff Dike wrote:
> 
> > > For some reason I now need this patch to make bk-current to compile 
> > That patch is against stock 2.5.42, so I don't make any guarantees about
> > bk-current.

I was seeing the same failure on 2.5.42.

> 
> 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.
> 
> > 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.
> 
> 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.

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
andmike@us.ibm.com

Re: [uml-devel] Re: uml-patch-2.5.42-1

[Jeff Dike]

green@namesys.com said:
> 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. 

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

Re: uml-patch-2.5.42-1

[Jeff Dike]

green@namesys.com said:
> 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". 

Correct on all counts.  Nice spotting.

> 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 

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