[openib-general] Re: [PATCH] uDAPL build fix for OS vendor variations of IA64_FETCHADD

[James Lentini]

Hi Arlin, 

A couple of questions:

> Index: dapl/udapl/linux/dapl_osd.h
> ===================================================================
> --- dapl/udapl/linux/dapl_osd.h	(revision 3541)
> +++ dapl/udapl/linux/dapl_osd.h	(working copy)
> @@ -83,7 +83,6 @@
>  #include <asm/system.h>
>  #endif
>  
> -
>  /* Useful debug definitions */
>  #ifndef STATIC
>  #define STATIC static
> @@ -156,13 +155,17 @@
>  #ifdef __ia64__
>      DAT_COUNT	old_value;
>  
> -#if OS_VERSION >= LINUX_VERSION(2,6)
> -    IA64_FETCHADD  (old_value,v,1,4,rel);
> +#ifndef REDHAT_EL4
> +#   if OS_RELEASE >= LINUX_VERSION(2,6)
> +	IA64_FETCHADD(old_value,v,1,4,rel);
> +#   else
> +	IA64_FETCHADD(old_value,v,1,4);
> +#   endif
>  #else
> -    IA64_FETCHADD  (old_value,v,1,4);
> +   IA64_FETCHADD(old_value,v,1,4);
>  #endif

Previously, if we were on Linux => 2.6, we used the 5 parameter 
version, otherwise we used the 4 parameter version. 

Why don't we continue to use the 5 parameter version if we are on 
Linux => 2.6 and not REHHAT_EL4?

>  
> -#else  /* !__ia64__ */
> +#else

Why remove /* !__ia64__ */?

>      __asm__ __volatile__ (
>  	"lock;" "incl %0"
>  	:"=m" (*v)
> @@ -184,13 +187,17 @@
>  #ifdef __ia64__
>      DAT_COUNT	old_value;
>  
> -#if OS_VERSION >= LINUX_VERSION(2,6)
> -    IA64_FETCHADD  (old_value,v,-1,4,rel);
> +#ifndef REDHAT_EL4
> +#   if OS_RELEASE >= LINUX_VERSION(2,6)
> +	IA64_FETCHADD(old_value,v,-1,4,rel);
> +#   else
> +	IA64_FETCHADD(old_value,v,-1,4);
> +#   endif
>  #else
> -    IA64_FETCHADD  (old_value,v,-1,4);
> +   IA64_FETCHADD(old_value,v,-1,4);

Why not continue to use the 5 parameter version if we are on
Linux => 2.6 and not REHHAT_EL4?

>  #endif
>  
> -#else  /* !__ia64__ */
> +#else

Why remove /* !__ia64__ */?

>      __asm__ __volatile__ (
>  	"lock;" "decl %0"
>  	:"=m" (*v)
> @@ -227,9 +234,11 @@
>       */
>  
>  #ifdef __ia64__
> -
> -current_value = ia64_cmpxchg("acq",v,match_value,new_value,4);
> -
> +#ifdef REDHAT_EL4
> +    current_value = ia64_cmpxchg("acq",v,match_value,new_value,4);
> +#else
> +    current_value = ia64_cmpxchg(acq,v,match_value,new_value,4);
> +#endif
>  #else
>      __asm__ __volatile__ (
>          "lock; cmpxchgl %1, %2"

Everything else looks good.