[uml-devel] TLS support - status - need for re-testing

[uml-devel] TLS support - status - need for re-testing

[Blaisorblade]

I've integrated "linux-unistd" from Jeff series, while leaving out 
"user-desc" (I've solved the problem another way, which should work on both 
host header set - we give our own definition for userspace files, and use our 
one for kernelspace, but give the resulting type the name of "user_desc_t").
Jeff, I've looked at all other patches _names_ and they're patches which I 
already have, so I shouldn't have lost any other changes.

Plus, I've just done a couple of other cleanups of the TLS code, removing dead 
code which was there for experiments; I've implemented 2.4 host detection (I 
hope I've not forgot to check the host_supports_tls variable anywhere), and 
I'm ripping out global-ldt-sem which I longly hated (and which isn't easy to 
avoid).

I've ripped it out by comparing header dependencies to i386; the result is 
that our additional dependency is from processor.h and ptrace.h on ldt.h, but 
they only depend on the user_desc declaration and not on the uml_ldt_t 
declaration (which is the one using the semaphore).

So I split the user_desc part away in a new header file, and this fixed the 
problem.

However, I tested seriously the new code and it doesn't work with apache2 
mpm-worker nor with simpler tests.... I hadn't run such tests recently, and I 
now see that the code does assumptions which are wrong on x86_64 hosts.

Has anyone got success or failure with TLS code on 32-bit machines on 64-bit 
hosts?

More important: has anyone tested that?
-- 
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 xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Antoine Martin]

> Has anyone got success or failure with TLS code on 32-bit machines on 64-bit 
> hosts?
I believe FC4 and FC5 x86 still fail with tls errors during fsck. Not
sure why. (images on my site)
All the other images were completely rebuilt in the last 2 months and
none of them have had their /lib/tls moved out of the way, so I assume
that glibc uses tls and all these images seem to work fine.

> More important: has anyone tested that?
I thought i had... but maybe not.

Antoine



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Jeff Dike]

On Thu, Mar 23, 2006 at 03:17:45AM +0100, Blaisorblade wrote:
> Jeff, I've looked at all other patches _names_ and they're patches which I 
> already have, so I shouldn't have lost any other changes.

I think I didn't change any of your patches - any changes I made got
their own patches, so you should be safe.

> Plus, I've just done a couple of other cleanups of the TLS code, removing dead 
> code which was there for experiments; I've implemented 2.4 host detection (I 
> hope I've not forgot to check the host_supports_tls variable anywhere), and 
> I'm ripping out global-ldt-sem which I longly hated (and which isn't easy to 
> avoid).

Do you have a set of patches which are candidates for akpm?  I'm
looking at 
	http://www.user-mode-linux.org/~blaisorblade/patches/guest/uml-2.6.15-bb6/broken-out/series

and that seems unsuitable, as some patches seem to reverse earlier
ones.

				Jeff


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Blaisorblade]

On Thursday 23 March 2006 22:14, Jeff Dike wrote:
> On Thu, Mar 23, 2006 at 03:17:45AM +0100, Blaisorblade wrote:

> Do you have a set of patches which are candidates for akpm?  I'm
> looking at
> 	http://www.user-mode-linux.org/~blaisorblade/patches/guest/uml-2.6.15-bb6/
>broken-out/series

> and that seems unsuitable, as some patches seem to reverse earlier
> ones.

Yes, that is true, but meanwhile give a testing to them, especially regression 
test the new patches. I can't merge new patches in until I'm sure they don't 
cause regressions.

ASAP I'll test them bypassing the "x86_64 different host numbers" bug, and if 
they work and no issue seems introduced I'll do the merge.

Meanwhile I could merge the initial stack without anything touching that extra 
load_TLS call in process_kern.c, but I hope to get to the real solution early 
enough.
-- 
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



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Jeff Dike]

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]

On Fri, Mar 24, 2006 at 12:44:05AM +0100, Blaisorblade wrote:
> Yes, that is true, but meanwhile give a testing to them, especially regressio
> test the new patches. I can't merge new patches in until I'm sure they don't 
> cause regressions.

OK, I give it a thumbs-up on a first boot - it's running my FC5
filesystem without any trouble.  I'll keep playing with it and let you
know of any trouble.

Attached is a tarball of the patches merged after the patches I have
planned for 2.6.17.  Nothing major, just some fuzz, some Makefile
rejections, and rejections from some whitespace cleanup in
kernel/skas/process_kern.c.

				Jeff


[-- Attachment #2: tls.tar --]
[-- Type: application/x-tar, Size: 112640 bytes --]

Re: [uml-devel] Re: TLS support - status - need for re-testing

[Antoine Martin]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

On Thu, 2006-03-23 at 20:51 -0500, Jeff Dike wrote:
> On Fri, Mar 24, 2006 at 12:44:05AM +0100, Blaisorblade wrote:
> > Yes, that is true, but meanwhile give a testing to them, especially regressio
> > test the new patches. I can't merge new patches in until I'm sure they don't 
> > cause regressions.
> 
> OK, I give it a thumbs-up on a first boot - it's running my FC5
> filesystem without any trouble.  I'll keep playing with it and let you
> know of any trouble.
I haven't been able to run FC4/FC5 x86 filesystems with any of the
patches floating around. (on amd64 host)

> Attached is a tarball of the patches merged after the patches I have
> planned for 2.6.17.  Nothing major, just some fuzz, some Makefile
> rejections, and rejections from some whitespace cleanup in
> kernel/skas/process_kern.c.
I couldn't figure out in which order I need to apply these (no series
file) and I've ran out of time.

Antoine

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: [uml-devel] Re: TLS support - status - need for re-testing

[Blaisorblade]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On Friday 24 March 2006 11:39, Antoine Martin wrote:
> On Thu, 2006-03-23 at 20:51 -0500, Jeff Dike wrote:
> > On Fri, Mar 24, 2006 at 12:44:05AM +0100, Blaisorblade wrote:
> > > Yes, that is true, but meanwhile give a testing to them, especially
> > > regressio test the new patches. I can't merge new patches in until I'm
> > > sure they don't cause regressions.
> >
> > OK, I give it a thumbs-up on a first boot - it's running my FC5
> > filesystem without any trouble.  I'll keep playing with it and let you
> > know of any trouble.
>
> I haven't been able to run FC4/FC5 x86 filesystems with any of the
> patches floating around. (on amd64 host)

I can inform you it's not FC4/FC5 but it's the amd64 host.

I missed this because I hadn't time for proper tests before, however:

add on top of 2.6.15-bb6 the attached patch. (It should more or less work 
against any other tls patchset, but I tested it like this).

Don't compile the resulting kernel for x86, and don't run the UML you get on 
x86 host!

As you can see (it touches include/asm-i386 only), this patch is an quick hack 
made for testing, but I know how to fix it, and I'm going to code it.

> > Attached is a tarball of the patches merged after the patches I have
> > planned for 2.6.17.  Nothing major, just some fuzz, some Makefile
> > rejections, and rejections from some whitespace cleanup in
> > kernel/skas/process_kern.c.

> I couldn't figure out in which order I need to apply these (no series
> file) and I've ran out of time.
Guess it's -bb6 series file (or I won't be able to apply them).
> Antoine

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

[-- Attachment #2: workaround-x86_64-host-problem --]
[-- Type: text/x-diff, Size: 521 bytes --]

Only for testing on x86_64 hosts.

Index: linux-2.6.15/include/asm-i386/segment.h
===================================================================
--- linux-2.6.15.orig/include/asm-i386/segment.h
+++ linux-2.6.15/include/asm-i386/segment.h
@@ -46,7 +46,7 @@
  *  31 - TSS for double fault handler
  */
 #define GDT_ENTRY_TLS_ENTRIES	3
-#define GDT_ENTRY_TLS_MIN	6
+#define GDT_ENTRY_TLS_MIN	12
 #define GDT_ENTRY_TLS_MAX 	(GDT_ENTRY_TLS_MIN + GDT_ENTRY_TLS_ENTRIES - 1)
 
 #define TLS_SIZE (GDT_ENTRY_TLS_ENTRIES * 8)

Re: [uml-devel] Re: TLS support - status - need for re-testing

[Blaisorblade]

On Friday 24 March 2006 02:51, Jeff Dike wrote:
> On Fri, Mar 24, 2006 at 12:44:05AM +0100, Blaisorblade wrote:
> > Yes, that is true, but meanwhile give a testing to them, especially
> > regressio test the new patches. I can't merge new patches in until I'm
> > sure they don't cause regressions.

> OK, I give it a thumbs-up on a first boot - it's running my FC5
> filesystem without any trouble.  I'll keep playing with it and let you
> know of any trouble.

With the x86-64 host hack, it works here too.

To do good stress-testing, I've actually noticed the existing problems when I 
started running apache benchmark (with apache using multithreading, i.e. 
mpm_worker_thread).

I run it like:

ab2 -n 100 -c 10 -t 30 -v 1 Sarge/

where -n, -c and -t are simple parameters which can be tuned at will. (It 
could be simply ab on your distro).

> Attached is a tarball of the patches merged after the patches I have
> planned for 2.6.17.  Nothing major, just some fuzz, some Makefile
> rejections, and rejections from some whitespace cleanup in
> kernel/skas/process_kern.c.

Will look at.
-- 
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



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Jeff Dike]

On Fri, Mar 24, 2006 at 12:44:05AM +0100, Blaisorblade wrote:
> Yes, that is true, but meanwhile give a testing to them, especially
> regression test the new patches. I can't merge new patches in until
> I'm sure they don't cause regressions.

I took a quick look through the patches - here are some comments:

uml-clean-arch_switch -

This chunk is strange:

@@ -141,7 +148,6 @@ static void new_thread_handler(int sig)
 	set_cmdline("(kernel thread)");
 
 	change_sig(SIGUSR1, 1);
-	change_sig(SIGVTALRM, 1);
 	change_sig(SIGPROF, 1);
 	local_irq_enable();
 	if(!run_kernel_thread(fn, arg, &current->thread.exec_buf))

If you're fixing a bug, you should say what it is.  But the original
looks right, since the process has been scheduled and is running on
its kernel stack, so it should be able to take timer interrupts now.

uml-add-tls-support -

copy_thread - OK for now, but we should move the CLONE_TLS bit into
arch code.

arch_ptrace - Do all arches have PTRACE_[GS]ET_THREAD_AREA, maybe move
inside an ifdef PTRACE_GET_THREAD_AREA

Any patches that change new files should be merged into this, I
think.  That basically means that the stack will mostly collapse into
this one patch.  I didn't see any patches which add new bits of
functionality which would make sense standalone.

global-ldt-sem - We should be using mutexes now, not semaphores

detect-2_4-host - is there no more direct way to detect TLS support?

undo-global-ldt-sem - what's so horrible about the global?  it won't
be highly contended, so making a single mutex saves memory from every
ldt.

				Jeff




-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Blaisorblade]

On Friday 24 March 2006 05:17, Jeff Dike wrote:
> On Fri, Mar 24, 2006 at 12:44:05AM +0100, Blaisorblade wrote:
> > Yes, that is true, but meanwhile give a testing to them, especially
> > regression test the new patches. I can't merge new patches in until
> > I'm sure they don't cause regressions.
>
> I took a quick look through the patches - here are some comments:
>
> uml-clean-arch_switch -
>
> This chunk is strange:
>
> @@ -141,7 +148,6 @@ static void new_thread_handler(int sig)
>  	set_cmdline("(kernel thread)");
>
>  	change_sig(SIGUSR1, 1);
> -	change_sig(SIGVTALRM, 1);
>  	change_sig(SIGPROF, 1);
>  	local_irq_enable();
>  	if(!run_kernel_thread(fn, arg, &current->thread.exec_buf))
>
> If you're fixing a bug, you should say what it is.

We discussed this time ago, and read the description of the patch:

# Also, as suggested by Jeff, remove a redundant enabling of SIGVTALRM,
# comprised in the subsequent local_irq_enable(). I'm just a bit dubious if
# ordering matters there...

Note this pre-dates soft interrupts, however.

> uml-add-tls-support -

> copy_thread - OK for now, but we should move the CLONE_TLS bit into
> arch code.

> arch_ptrace - Do all arches have PTRACE_[GS]ET_THREAD_AREA, maybe move
> inside an ifdef PTRACE_GET_THREAD_AREA

No, only x86. x86_64 already is different (supports that only for emulated 
processes).

> Any patches that change new files should be merged into this, I
> think.  That basically means that the stack will mostly collapse into
> this one patch.  I didn't see any patches which add new bits of
> functionality which would make sense standalone.

> global-ldt-sem - We should be using mutexes now, not semaphores

That's your patch, but below I drop this.

> detect-2_4-host - is there no more direct way to detect TLS support?

Yes, there is. I'll fix this when I do x86/x86_64 host distinction.

> undo-global-ldt-sem - what's so horrible about the global?  it won't
> be highly contended, so making a single mutex saves memory from every
> ldt.

For memory: 500 existing processes (a huge number in practice) * 20 byte (and 
I think a semaphore is smaller) = 10 Kbyte. We waste more for the kernel 
stack of 1 thread.

For reason: the compilation problem can be avoided differently (and the 
subsequent patch also merges duplicated code).

I didn't like global_ldt_sem because I felt it unclean; if done for memory 
usage it's another story, but still we need to check this creates no locking 
problem (almost surely no, but I want to look at the code well).
-- 
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 xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Jeff Dike]

On Fri, Mar 24, 2006 at 12:43:33PM +0100, Blaisorblade wrote:
> We discussed this time ago, and read the description of the patch:
> 
> # Also, as suggested by Jeff, remove a redundant enabling of SIGVTALRM,
> # comprised in the subsequent local_irq_enable(). I'm just a bit dubious if
> # ordering matters there...

Oops, nevermind.

> Note this pre-dates soft interrupts, however.

Shouldn't matter.

> > global-ldt-sem - We should be using mutexes now, not semaphores
> 
> That's your patch, but below I drop this.

My patch predates mutexes.

But you still use semaphores, and that should be updated.

> > undo-global-ldt-sem - what's so horrible about the global?  it won't
> > be highly contended, so making a single mutex saves memory from every
> > ldt.
> 
> For memory: 500 existing processes (a huge number in practice) * 20 byte (and
> I think a semaphore is smaller) = 10 Kbyte. We waste more for the kernel 
> stack of 1 thread.
> 
> For reason: the compilation problem can be avoided differently (and the 
> subsequent patch also merges duplicated code).
> 
> I didn't like global_ldt_sem because I felt it unclean; if done for memory 
> usage it's another story, but still we need to check this creates no locking 
> problem (almost surely no, but I want to look at the code well).

It's simpler.  A single global lock is simpler than a lock in every 
datastructure.

				Jeff



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
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] Re: TLS support - status - need for re-testing

[Blaisorblade]

On Friday 24 March 2006 18:32, Jeff Dike wrote:
> On Fri, Mar 24, 2006 at 12:43:33PM +0100, Blaisorblade wrote:
> > We discussed this time ago, and read the description of the patch:

> > > global-ldt-sem - We should be using mutexes now, not semaphores
> >
> > That's your patch, but below I drop this.
>
> My patch predates mutexes.
>
> But you still use semaphores, and that should be updated.

We'll do it at some time, there's no hurry for that; or they'll do it anyway 
before ripping out semaphores.

> It's simpler.  A single global lock is simpler than a lock in every
> datastructure.

My only doubt (I'd almost swear it doesn't happen) is if two locks of two 
different structures are ever taken. That would fail.

About locking, I've discovered that sigio_lock must use spin_lock_irqsave() 
instead of spin_lock; currently with Mutex debugging I get hangs on that 
spinlock, but on UP currently you get a race between process context and 
interrupt context.

The failing case is when that lock is taken in process context, then an 
interrupt is triggered whose handler takes again that. In fact, for mutual 
exclusion between process context and irq context an spin_lock_irq{,save} is 
_always_ recommended.

Only problem will be to check for sleepers in process context - there could be 
many ones.
-- 
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 xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel