RE: InfiniBand/RDMA merge plans for 2.6.25

[Glenn Streiff]

> From: Roland Dreier [mailto:rdreier@xxxxxxxxx]
> To: Christoph Hellwig; Glenn Streiff 
> 
> Rather than arguing over whether we have to have sparse clean code, I
> decided to annotate the code myself.  Here's a patch that fixes most
> of the sparse warnings in the nes driver.  There's still some stuff
> that actually looks buggy, like the way hte_index stuff is handled.
> You initialize hte_index_mask as:
> 
> 	hte_index_mask = ((u32)1 << ((u32temp & 0x001f)+1))-1;
> 	nesadapter->hte_index_mask = hte_index_mask;
> 
> but then compute hte_index stuff with:
> 
> 	nesqp->hte_index = cpu_to_be32(
> 			crc32c(~0, (void *)&nes_quad, 
> sizeof(nes_quad)) ^ 0xffffffff);
> 
> and then do:
> 
> 	nesqp->hte_index &= nesadapter->hte_index_mask;
> 
> which seems odd to say the least (hte_index is big-endian,
> hte_index_mask is cpu-endian).
> 
> And also, there's code with the loc_addr/rem_addr etc that seem very
> confused.  For example
> 
> 	cm_info->loc_addr = htonl(cm_info->loc_addr);
> 	cm_info->rem_addr = htonl(cm_info->rem_addr);
> 	cm_info->loc_port = htons(cm_info->loc_port);
> 	cm_info->rem_port = htons(cm_info->rem_port);
> 
> which is obviously impossible to annotate correctly, and I couldn't
> keep track of the endianness stuff elsewhere.

Thanks for the additional review and patch.  I take your point.  

The part is little endian and the driver is functional for little and big endian 
platforms.  There may have been some expedience with the declarations there.  
I think it can be improved.  Let me take it up with the person who wrote that code.

Also, I want everyone to understand that my skill set is weighted more
towards build/install/config.  And I guess I'll be patch wrangling as well.
So I'll rely on input from my developers for issues that drill down or
I'll have them post directly.  I respect the work you guys do.

For now, let me get some qa cycles with your patch across x86_64 and power
(and probably a couple others).

Regards,

Glenn

> 
> Anyway this is what I have in case the promised cleanups don't turn up
> in time...
> 
> Signed-off-by: Roland Dreier <rolandd@xxxxxxxxx>
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/