2002-10-14 23:48:05

by Jeff Dike

[permalink] [raw]
Subject: 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


2002-10-15 06:36:28

by Oleg Drokin

[permalink] [raw]
Subject: Re: uml-patch-2.5.42-1

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,

2002-10-15 07:19:38

by Mike Anderson

[permalink] [raw]
Subject: Re: [uml-devel] Re: uml-patch-2.5.42-1

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 [[email protected]] 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
-andmike
--
Michael Anderson
[email protected]

2002-10-15 11:55:07

by Oleg Drokin

[permalink] [raw]
Subject: Re: uml-patch-2.5.42-1

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);
}

2002-10-15 12:42:49

by Jeff Dike

[permalink] [raw]
Subject: Re: uml-patch-2.5.42-1

[email protected] 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

2002-10-15 13:00:11

by Oleg Drokin

[permalink] [raw]
Subject: Re: uml-patch-2.5.42-1

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

2002-10-15 13:53:41

by Mike Anderson

[permalink] [raw]
Subject: Re: [uml-devel] Re: uml-patch-2.5.42-1

Oleg Drokin [[email protected]] 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
[email protected]

2002-10-15 14:42:08

by Jeff Dike

[permalink] [raw]
Subject: Re: [uml-devel] Re: uml-patch-2.5.42-1

[email protected] 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

2002-10-15 15:15:49

by Jeff Dike

[permalink] [raw]
Subject: Re: uml-patch-2.5.42-1

[email protected] 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