Roland Dreier <[EMAIL PROTECTED]> wrote on 01/30/2008 03:05:04 PM: > Looks almost identical to what I came up with (below), except: > > > + shift = __ffs((unsigned long) ((mask & ((1ull<<31)-1)) | (1ull<<31))); > > I think there's no reason to mask off the top bit of mask if we're > going to set it immediately;
I was actually masking off the top 33 bits before setting bit 31. I agree that it's unnecessary, but I wanted it to be clear that the possible truncation from 64 to 32 bits (on 32-bit target machines) is okay. I'd have left off the masking and the cast if the kernel provided something like __ffsll() or __ffs64() that would take 64-bit arguments even on 32-bit machines, but it doesn't as far as I can tell. You've changed mask itself into an unsigned long: > - u64 mask; > + unsigned long mask; so the truncation (on 32-bit machines) is now happening as the mask is constructed. In this case the truncation is okay, because ultimately you only care about the low-order bits anyway, but implicit truncations always raise a red flag for me. In any case, I think your patch does the job and does it efficiently. - Bryan
_______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
