linux-kernel - Re: ALIGN (Re: [PATCH] Fix get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070310003140.GC17084@flower.upol.cz>
Date:	Sat, 10 Mar 2007 01:31:41 +0100
From:	Oleg Verych <olecom@...wer.upol.cz>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	David Howells <dhowells@...hat.com>,
	Al Viro <viro@....linux.org.uk>, akpm@...ux-foundation.org,
	benh@...nel.crashing.org, linux-kernel@...r.kernel.org,
	hpa@...or.com, johannes@...solutions.net
Subject: Re: ALIGN (Re: [PATCH] Fix get_order())

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/