Hi Christos, Thanks for reviewing my patch.
(2014/01/22 2:56), Christos Zoulas wrote: > In article <[email protected]>, > Ryota Ozaki <[email protected]> wrote: >> Hi, >> >> I'm working on porting DTrace to ARM. > > Nicely done: > 1. there seem to be some whitespace only changes Sorry for the messy code. Will fix. > 2. what's the STRONG_ALIAS to __ffssi2 about? This is needed to modload solaris. Without the fix I got the following error: # modload solaris kobj_checksyms, 880: [solaris]: linker error: symbol `__ffssi2' not found WARNING: module error: unable to affix module `solaris', error 8 modload: Exec format error This problem seems to be not dtrace specific. Other modules, e.g., nand, are also encountered the error in the case of evbarm/BEAGLEBONE. I want someone to confirm the problem on an evbarm/BEAGLEBONE machine (and other evbarm/arm machines). I think this fix should be a separate patch if the fix is correct. > 3. what about the deleted code in dtrace_debug.c The deletion is because of replacing dtrace_cmpset_long with atomic_cas_ulong. Please see the reply to 5. for more discussion. > 4. I am torn about the cpuid -> cpu_id change. In my version of the > changes, I had fixed the arm code instead in sys/arch/arm. It was > used in very few places there. Sure. I changed cpuid -> cpu_id because I thought I should reduce the changes of the kernel core code. So if the change of the arm code is acceptable, I will do it in my next patch. > 5. I think that the dtrace_cmpset_long -> atomic_cas_ulong change is > questionable (because we probably want to keep the ABI's dtrace uses > stable; perhaps just making this a define in some header file and > leaving the source alone? dtrace_cmpset_long is used only in dtrace_debug.c and I think the change doesn't affect the dtrace ABIs. ...just now I found the change on FreeBSD http://svnweb.freebsd.org/base/head/sys/cddl/dev/dtrace/dtrace_debug.c?r1=209059&r2=226452 ozaki-r > > christos >
