Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
On Monday 03 October 2005 15:40, Allan Graves wrote: I'm not understanding why glibc would break the registers at will. From setjmp: /* NOTE: The machine-dependent definitions of `__sigsetjmp' assume that a `jmp_buf' begins with a `__jmp_buf' and that `__mask_was_saved' follows it. Do not move these members or add others before it. */ Seems to indicate to me that this isn't gonna change, the comments above are mostly relative to the definitions of __sigsetjmp, i.e. to glibc internal code. Also, it's related to the layout of the structure, while our main problem is not the structure layout. and I'm using the bits/setjmp.h defines, so if they do change, the code should just follow along with the change. Am I missing something? I didn't even see those constants - I stopped earlier, at __jmp_buf_tag. Given that it feels like a Glibc private thing, I worried. Since those constants are explicitly exported, I guess that's for userspace programs as well, so Glibc provides that API as a public one. -- 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! Mail: gratis 1GB per i messaggi e allegati da 10MB http://mail.yahoo.it --- This SF.Net email is sponsored by: Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
On Sun, Oct 02, 2005 at 12:31:04PM +0200, Blaisorblade wrote: Yeah, it's bad. The other way to do it is to explictly save the registers in the thread struct, which is effectively the reimplementing setjmp option which you mentioned. At least, if we save them separately from the jmpbuf_t, we can use them for sysrq t, without reimplementing setjmp() and longjmp(). Not nice, wastes 24 bytes, but would work. Yup. I have the doubt that the location of those registers is part of the ABI, I think they are probably not. Probably, however, it's just better to test on, say, a Slackware 8.1, and hope for the best and go doing a fix when things change. Yeah, I agree. On the positive side, I think that jmp_buf has probably been unchanged for a long time, and will remain so. Jeff --- This SF.Net email is sponsored by: Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
[uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
I've recently realized how potentially misguided that thing is. And I am wondering about whether the recent eactivate_all_fds failed, errno = 9 (no, the initial d never comes out) may be due to this. Even if, now I see that was done even before... but first only on TT mode, then also in SKAS mode with commit 6770cb61ff6d557613a8382b28f9b0a919fb112f, which says when getting a fatal signal. But still, Jeff, how can we expect that malloc won't stomp over all our data which we preallocated with kmalloc and such? There's no single mention of that in your original changelog, and this is untrivial, so I can assume you didn't realize this issue. The git commit is 026549d28469f7d4ca7e5a4707f0d2dc4f2c164c. On the other side, could you explain why you don't like kmalloc in first place? It surely works. Also, there are some calls to kmalloc in the shutdown path - and they work. I know this because I saw a problem with one of them: it gave might_sleep while atomic, and it was kmalloc in the shutdown, or rather, in panic() - for the broken sysrq t (where's the fix you promised?). While looking for the change function, I thought even if it's static, that's still not enough to deserve such an insignificant name. Ah, it's in arch/um/drivers/net_user.c. 3Debug: sleeping function called from invalid context at /home/paolo/Admin/kernel/6/VCS/linux-2.6.13/mm/slab.c:2096 in_atomic():1, irqs_disabled():0 a0327088: [a00164f2] dump_stack+0x22/0x30 a03270a8: [a0049e1c] __might_sleep+0xac/0xd0 a03270c8: [a0094fa0] __kmalloc+0xc0/0x110 a03270f8: [a00129ba] um_kmalloc+0x1a/0x20 a0327108: [a0038b6b] change+0xcb/0x220 a03271d8: [a0038d10] close_addr+0x20/0x30 a03271e8: [a00385bb] iter_addresses+0x7b/0x90 a0327218: [a0041b0a] tuntap_close+0x4a/0x70 a0327238: [a003827d] close_devices+0x4d/0x90 a0327258: [a0012a92] do_uml_exitcalls+0x22/0x40 a0327268: [a0013763] uml_cleanup+0x13/0x20 panic... The real solution for this warning is to replace um_kmalloc with malloc(), and set, during shutdown, kmalloc_only_atomic - which would switch __wrap_malloc() from um_kmalloc to um_kmalloc_atomic. Or better yet, simply test in_atomic() and irqs_disabled() to choose between the atomic and normal versions. -- 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! Mail: gratis 1GB per i messaggi e allegati da 10MB http://mail.yahoo.it --- This SF.Net email is sponsored by: Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl ___ User-mode-linux-devel mailing list User-mode-linux-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
On Wed, Sep 28, 2005 at 01:46:15PM +0200, Blaisorblade wrote: Also, there are some calls to kmalloc in the shutdown path - and they work. I know this because I saw a problem with one of them: it gave might_sleep while atomic, and it was kmalloc in the shutdown, or rather, in panic() - for the broken sysrq t (where's the fix you promised?). Attached. Jeff # From Allan Graves: # # Fix sysrq-t support for skas mode. The old code had the IP and SP coming # from the registers in the thread struct, which are completely wrong since # those are the userspace registers. This fixes that by pulling the correct # values from the jmp_buf in which the kernel state of each thread is stored. # # Signed-off-by: Allan Graves [EMAIL PROTECTED] Index: test/arch/um/include/registers.h === --- test.orig/arch/um/include/registers.h 2005-09-14 15:52:06.0 -0400 +++ test/arch/um/include/registers.h2005-09-27 19:00:35.0 -0400 @@ -15,16 +15,6 @@ extern void restore_registers(int pid, union uml_pt_regs *regs); extern void init_registers(int pid); extern void get_safe_registers(unsigned long * regs); +extern void get_thread_regs(union uml_pt_regs *uml_regs, void *buffer); #endif - -/* - * Overrides for Emacs so that we follow Linus's tabbing style. - * Emacs will notice this stuff at the end of the file and automatically - * adjust the settings for this buffer only. This must remain at the end - * of the file. - * --- - * Local variables: - * c-file-style: linux - * End: - */ Index: test/arch/um/include/sysdep-x86_64/ptrace.h === --- test.orig/arch/um/include/sysdep-x86_64/ptrace.h2005-09-27 11:33:43.0 -0400 +++ test/arch/um/include/sysdep-x86_64/ptrace.h 2005-09-27 19:55:07.0 -0400 @@ -218,10 +218,6 @@ case RBP: UPT_RBP(regs) = __upt_val; break; \ case ORIG_RAX: UPT_ORIG_RAX(regs) = __upt_val; break; \ case CS: UPT_CS(regs) = __upt_val; break; \ -case DS: UPT_DS(regs) = __upt_val; break; \ -case ES: UPT_ES(regs) = __upt_val; break; \ -case FS: UPT_FS(regs) = __upt_val; break; \ -case GS: UPT_GS(regs) = __upt_val; break; \ case EFLAGS: UPT_EFLAGS(regs) = __upt_val; break; \ default : \ panic(Bad register in UPT_SET : %d\n, reg); \ Index: test/arch/um/kernel/sysrq.c === --- test.orig/arch/um/kernel/sysrq.c2005-06-17 15:48:29.0 -0400 +++ test/arch/um/kernel/sysrq.c 2005-09-27 19:00:35.0 -0400 @@ -62,13 +62,7 @@ if (esp == NULL) { if (task != current task != NULL) { - /* XXX: Isn't this bogus? I.e. isn't this the -* *userspace* stack of this task? If not so, use this -* even when task == current (as in i386). -*/ esp = (unsigned long *) KSTK_ESP(task); - /* Which one? No actual difference - just coding style.*/ - //esp = (unsigned long *) PT_REGS_IP(task-thread.regs); } else { esp = (unsigned long *) esp; } @@ -84,5 +78,5 @@ } printk(Call Trace: \n); - show_trace(current, esp); + show_trace(task, esp); } Index: test/arch/um/os-Linux/sys-i386/registers.c === --- test.orig/arch/um/os-Linux/sys-i386/registers.c 2005-09-14 15:52:06.0 -0400 +++ test/arch/um/os-Linux/sys-i386/registers.c 2005-09-27 19:28:43.0 -0400 @@ -5,6 +5,7 @@ #include errno.h #include string.h +#include setjmp.h #include sysdep/ptrace_user.h #include sysdep/ptrace.h #include uml-config.h @@ -126,13 +127,11 @@ memcpy(regs, exec_regs, HOST_FRAME_SIZE * sizeof(unsigned long)); } -/* - * Overrides for Emacs so that we follow Linus's tabbing style. - * Emacs will notice this stuff at the end of the file and automatically - * adjust the settings for this buffer only. This must remain at the end - * of the file. - * --- - * Local variables: - * c-file-style: linux - * End: - */ +void get_thread_regs(union uml_pt_regs *uml_regs, void *buffer) +{ + struct __jmp_buf_tag *jmpbuf = buffer; + + UPT_SET(uml_regs, EIP, jmpbuf-__jmpbuf[JB_PC]); + UPT_SET(uml_regs, UESP, jmpbuf-__jmpbuf[JB_SP]); + UPT_SET(uml_regs, EBP, jmpbuf-__jmpbuf[JB_BP]); +} Index: test/arch/um/os-Linux/sys-x86_64/registers.c === ---