[openib-general] [PATCH] SDP warnings on x86_64

[Grant Grundler]

On Fri, Feb 11, 2005 at 06:40:58PM -0800, Libor Michalek wrote:
>   If no one objects, a patch to clean up compile warnings on x86_64.

I haven't applied this patch yet - I read mail on the other side of
a firewall where my machines are. But I don't think it will fix
the compile error below.

I'm not thrilled about the number of "(void *)(unsigned long)" casts.
A small inline function to convert (int) to (void *) would be cleaner.
E.g. (not compiled):
static inline void * hash_arg(int x) { return (void *)(unsigned long) x; }



With rev 1781, on ia64 linux-2.6.11-rc3, I'm getting the expected warnings
and one error:
drivers/infiniband/ulp/sdp/sdp_iocb.c: In function `_sdp_iocb_page_save':
drivers/infiniband/ulp/sdp/sdp_iocb.c:194: error: request for member `pgd' in something not a structure or union

The offending line:
                pmd = pmd_offset(pgd, addr);

pmd_offset() is defined in include/asm/pgtable.h:

/* Find an entry in the second-level page table.. */
#define pmd_offset(dir,addr) \
        ((pmd_t *) pud_page(*(dir)) + (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1)))

#define pud_page(pud) ((unsigned long) __va(pud_val(pud) & _PFN_MASK))

and ia64 uses asm-generic for the last bit:
include/asm-generic/pgtable-nopud.h:#define pud_val(x) (pgd_val((x).pgd))

include/asm/page.h:# define pgd_val(x)  ((x).pgd)


But the offending line is getting expanded to:
            pmd = ((pmd_t *) ((unsigned long) ({ia64_va _v; _v.l = (long) (((((*(pgd)).pgd).pgd)) & (((((unsigned long)(1)) << 50) - 1) & ~0xfffUL)); _v.f.reg = -1; _v.p;})) + (((addr) >> (14 + (14 -3))) & ((1UL << (14 -3)) - 1)));

i.e. ((((pgd)).pgd).pgd)  - one too many since asm/page.h defines it as:

  typedef struct { unsigned long pgd; } pgd_t;


I'm guessing _sdp_iocb_page_save() is just broken.
The ia64 kernel builds fine.
And every other use of pmd_offset() looks like this:
	mm/rmap.c:      pmd = pmd_offset(pud, address);

ie. they are passing in pud_t * instead of pgd_t *.
Someone care to enlighten me on which usage is correct?


One more annoyance: sdp_iocb.c only includes one local file (sdp_main.h).
That's more than I can take on tonight.

And sdp_main.h violates one of the kernel include file rules:
	include asm/ headers *after* linux/ headers.

I'd like to submit a patch for the include file ordering
but can't build right now on ia64.

thanks,
grant