Re: sablevm.processor.affinity patch

I'm also bumping this one for review / committal.

Chris

Chris Pickett wrote:
> Grzegorz B. Prokopski wrote:
> 
>> On Wed, 2004-11-17 at 21:14, Chris Pickett wrote:
>>
>>> Here is my initial crack at support for processor affinity in SableVM. 
>>
>>
>>
>> Thanks Chris, this looks fairly good.  Some remarks:
>>
>> - all this stuff has to use #ifdef __linux__ around C code
> 
> 
> okay.  conceivably, this should work one day on other kernels, so the
> #ifdef'd portion is minimal.  i left the property parsing and function 
> calls in there for all systems, and simply print out an error message if 
> the user tries to set the property to anything but -1 if __linux__ and 
> __NR_sched_setaffinity are not defined.  what about when we check for 
> new header files in configure.ac?  are syscall.h and sched.h standard 
> files?  or does that need to be protected too?
> 
>> - on your test system (SMP Gentoo Linux?), does the normal
>> sched_setaffinity() call work or not?  What is its version of glibc?
> 
> 
> yes, it works.
> 
> magic ~/sablevm-setaffinity $ /lib/libc.so.6
> GNU C Library 20040808 release version 2.3.4, by Roland McGrath et al.
> Copyright (C) 2004 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.
> There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
> PARTICULAR PURPOSE.
> Compiled by GNU CC version 3.3.4 20040623 (Gentoo Linux 3.3.4-r2,
> ssp-3.3.2-2, pie-8.7.6).
> Compiled on a Linux 2.4.21 system on 2004-11-08.
> Available extensions:
>          GNU libio by Per Bothner
>          crypt add-on version 2.1 by Michael Glad and others
>          linuxthreads-0.10 by Xavier Leroy
>          BIND-8.2.3-T5B
>          libthread_db work sponsored by Alpha Processor Inc
>          NIS(YP)/NIS+ NSS modules 0.19 by Thorsten Kukuk
> Thread-local storage support included.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/libc/bugs.html>.
> 
> However, it did not work as recent as the end of June 29th.  I guess
> it's a good way to force people to encourage people to upgrade, so I've
> made the glibc interface be the default.  People will only get warning
> messages if they've explicitly set the processor.affinity option and
> don't have the right glibc.
> 
>> - if you need to directly use the syscall() you should have
>> #ifdef __NR_sched_setaffinity or maybe rather traditional
>> #ifdef SYS_sched_setaffinity.  Otherwise the compilation would fail
>> on systems that do not have it defined.
>>
>> - I think that for now, until SMP issues get fixed, we should use
>> sysconf() to detect no. of CPUs and if it's bigger than 1 try to set
>> affinity to a randomly choosen CPU.  There should be an informative
>> message about it happening outputted while this workaround for SMP
>> problems is activated.
> 
> 
> I disagree.  The default should be to use all processors.  If people
> find bugs, great, they can report them -- and we have an immediate fix
> available in the form of this processor affinity option; otherwise they
> won't find bugs at all.  SMP support is broken, but it's not _that_ 
> broken.  Also, your suggestion only works for Linux 2.5.8+ SMP machines, 
> and so if used, the default behaviour wouldn't be consistent.
> 
>> - looks like the vm->vprintf is assigned fairly early in
>> JNI_CreateJavaVM, so you should be able to use _svmf_printf() for
>> printing out any messages (and IMO they should go to stderr, as they're
>> not part of normal program output).
> 
> 
> _svmf_printf wants an _svmt_JNIEnv *, so I have to wait for the env to 
> be created and the vm assigned to it.  not a problem, really.
> 
>> I am not convinced we really need a configure option for all that. 
>> Checking whether SYS_sched_affinity is defined should be enough to
>> ensure that sched_setaffinity() call exists at all.  And calling
>> syscall() (on Linux) in the worst case will fail which is non-critical
>> failure.  Thus where possible - we can compile this code in, where not
>> - we'll automatically disable it using #ifdefs.
> 
> 
> okay.  I didn't know about __linux__.  there's something about a 
> __USE_GNU constant in /usr/include/sched.h, I'm not sure what to do 
> about it -- currently not defining it gives an "implicit definition of 
> sched_setaffinity" warning at compile-time (but everything still works).
> 
>> Chris, can you prepare an updated patch?
> 
> 
> svn diff -r3021:3065
> svn+ssh://svn.sablevm.org/public/developers/chris/sandbox/setaffinity/
> 
> Currently, if the user specifies an invalid mask (e.g. 16 on a 4-CPU 
> machine, which translates to 10000 (use only the 5th processor)), I get 
> the following:
> 
>  * setting processor affinity 16 for process 26448 failed
>  * using default value of -1 (all processors set)
> Command terminated by signal 9
> 
> I think that's okay.  More gracious error handling would require 
> checking errno after sched_setaffinity returns.
> 
> Cheers, and thanks,
> Chris
> 
> _______________________________________________
> SableVM-devel mailing list
> [email protected]
> http://sablevm.org/lists/control/listinfo/sablevm-devel
>