On Fri, Mar 09, 2007 at 03:15:10PM -0800, Linus Torvalds wrote:
>
>
> On Sat, 10 Mar 2007, Oleg Verych wrote:
> >
> > OTOH, if i would write it this way
> >
> > #define BALIGN(x,bits) ((((x) >> (bits)) + 1) << (bits))
>
> But that's *wrong*. It aligns something that is *already* aligned to
> something else.
Indeed. I'm confused by semantics of ALIGN macro, as from C arithmetic
side, as from using side. Former confusion also yields patches like
(Fix 'ALIGN()' macro, take 2), fixing that, i was wonder about.
BALIGN is like do-this alignment, and must be
void do_align(what, bits)
instead. While clear arithmetic (optimized in assembler, shifts are
C shifts!), it fails in value(alignment(what, how)) kind of thing.
> So you'd have to do it as something like
>
> #define ALIGN_TO_POW2(x,pow) \
> ((((x)+(1<<(pow))-1)>>(pow))<<(pow))
>
> and the thing is, that's actually both (a) less readable than what ALIGN()
> already does (b) unless the compiler notices that what you really want is
> a "bitwise and", it's really inefficient too because shifts are generally
> much more expensive than simple bitops.
>
> So you're simply better off doing it as
>
> #define ALIGN_TO_POW2(x,pow) ALIGN(x, (1ull << pow))
>
> but then you'd still have to depend on the "typeof()" magic in ALIGN() to
> turn the final end result back to the type of "x".
>
> (the "1ull" is necessary exactly because you don't know what type "x" is
> beforehand, so you need to make sure that the mask is at *least* as big a
> type as x, and not overflow to undefined C semantics).
Via typeof() *feature* and 1U, 1UL, 1ULL *things*, i (we?) have that, what
is described above.
Examples of using ALIGN. As that, i've picked earlier,
arch/powerpc/mm/hugetlbpage.c: addr =
ALIGN(addr+1,1UL<<HTLB_AREA_SHIFT);
arch/powerpc/mm/hugetlbpage.c: addr =
ALIGN(addr+1,1<<SID_SHIFT);
mixes two things, and i can't understand what is meant here,
lib/swiotlb.c: io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
lib/swiotlb.c: io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
lib/swiotlb.c: io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
clear "do align", etc.
So, maybe spitting "do align" (with C shifts, without C arithmetics
error-prone-due-to-C magic) and "alignment" isn't a bad idea.
Easiness is what i wanted to have. For example, like in this clean up,
'proper memory-mapped accesses rather than making up our own "volatile"
pointers' in 6c0ffb9d2fd987c79c6cbb81c3f3011c63749b1a. Clear C-vs-call()
rewrite.
Sorry, if i'm barking up the wrong tree here.
____
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
