提交 5318a29c 编写于 作者: B Boaz Harrosh 提交者: Trond Myklebust

pnfs-obj: Uglify objio_segment allocation for the sake of the principle :-(

At some past instance Linus Trovalds wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
> commit a84a79e4 upstream.
>
> The size is always valid, but variable-length arrays generate worse code
> for no good reason (unless the function happens to be inlined and the
> compiler sees the length for the simple constant it is).
>
> Also, there seems to be some code generation problem on POWER, where
> Henrik Bakken reports that register r28 can get corrupted under some
> subtle circumstances (interrupt happening at the wrong time?).  That all
> indicates some seriously broken compiler issues, but since variable
> length arrays are bad regardless, there's little point in trying to
> chase it down.
>
> "Just don't do that, then".

Since then any use of "variable length arrays" has become blasphemous.
Even in perfectly good, beautiful, perfectly safe code like the one
below where the variable length arrays are only used as a sizeof()
parameter, for type-safe dynamic structure allocations. GCC is not
executing any stack allocation code.

I have produced a small file which defines two functions main1(unsigned numdevs)
and main2(unsigned numdevs). main1 uses code as before with call to malloc
and main2 uses code as of after this patch. I compiled it as:
	gcc -O2 -S see_asm.c
and here is what I get:

<see_asm.s>
main1:
.LFB7:
	.cfi_startproc
	mov	%edi, %edi
	leaq	4(%rdi,%rdi), %rdi
	salq	$3, %rdi
	jmp	malloc
	.cfi_endproc
.LFE7:
	.size	main1, .-main1
	.p2align 4,,15
	.globl	main2
	.type	main2, @function
main2:
.LFB8:
	.cfi_startproc
	mov	%edi, %edi
	addq	$2, %rdi
	salq	$4, %rdi
	jmp	malloc
	.cfi_endproc
.LFE8:
	.size	main2, .-main2
	.section	.text.startup,"ax",@progbits
	.p2align 4,,15
</see_asm.s>

*Exact* same code !!!

So please seriously consider not accepting this patch and leave the
perfectly good code intact.

CC: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: NBoaz Harrosh <bharrosh@panasas.com>
Signed-off-by: NTrond Myklebust <Trond.Myklebust@netapp.com>
上级 e138ead7
...@@ -205,25 +205,36 @@ static void copy_single_comp(struct ore_components *oc, unsigned c, ...@@ -205,25 +205,36 @@ static void copy_single_comp(struct ore_components *oc, unsigned c,
int __alloc_objio_seg(unsigned numdevs, gfp_t gfp_flags, int __alloc_objio_seg(unsigned numdevs, gfp_t gfp_flags,
struct objio_segment **pseg) struct objio_segment **pseg)
{ {
struct __alloc_objio_segment { /* This is the in memory structure of the objio_segment
struct objio_segment olseg; *
struct ore_dev *ods[numdevs]; * struct __alloc_objio_segment {
struct ore_comp comps[numdevs]; * struct objio_segment olseg;
} *aolseg; * struct ore_dev *ods[numdevs];
* struct ore_comp comps[numdevs];
aolseg = kzalloc(sizeof(*aolseg), gfp_flags); * } *aolseg;
if (unlikely(!aolseg)) { * NOTE: The code as above compiles and runs perfectly. It is elegant,
* type safe and compact. At some Past time Linus has decided he does not
* like variable length arrays, For the sake of this principal we uglify
* the code as below.
*/
struct objio_segment *lseg;
size_t lseg_size = sizeof(*lseg) +
numdevs * sizeof(lseg->oc.ods[0]) +
numdevs * sizeof(*lseg->oc.comps);
lseg = kzalloc(lseg_size, gfp_flags);
if (unlikely(!lseg)) {
dprintk("%s: Faild allocation numdevs=%d size=%zd\n", __func__, dprintk("%s: Faild allocation numdevs=%d size=%zd\n", __func__,
numdevs, sizeof(*aolseg)); numdevs, lseg_size);
return -ENOMEM; return -ENOMEM;
} }
aolseg->olseg.oc.numdevs = numdevs; lseg->oc.numdevs = numdevs;
aolseg->olseg.oc.single_comp = EC_MULTPLE_COMPS; lseg->oc.single_comp = EC_MULTPLE_COMPS;
aolseg->olseg.oc.comps = aolseg->comps; lseg->oc.ods = (void *)(lseg + 1);
aolseg->olseg.oc.ods = aolseg->ods; lseg->oc.comps = (void *)(lseg->oc.ods + numdevs);
*pseg = &aolseg->olseg; *pseg = lseg;
return 0; return 0;
} }
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册