aboutsummaryrefslogtreecommitdiffstats
path: root/fs/nfs/objlayout/objio_osd.c
diff options
context:
space:
mode:
authorBoaz Harrosh <bharrosh@panasas.com>2012-03-13 23:44:26 -0400
committerTrond Myklebust <Trond.Myklebust@netapp.com>2012-03-13 23:47:59 -0400
commit5318a29c1943e9719e71495db6efb6fc084a45a9 (patch)
treeb274e3c6c2edc30abf2a0fc8d10eabffbf9598e1 /fs/nfs/objlayout/objio_osd.c
parente138ead73f872559778bb0c326e795206f96d3ce (diff)
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 a84a79e4d369a73c0130b5858199e949432da4c6 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: Boaz Harrosh <bharrosh@panasas.com> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Diffstat (limited to 'fs/nfs/objlayout/objio_osd.c')
-rw-r--r--fs/nfs/objlayout/objio_osd.c39
1 files changed, 25 insertions, 14 deletions
diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
index 405a62bdb9b4..3a621a2fd321 100644
--- a/fs/nfs/objlayout/objio_osd.c
+++ b/fs/nfs/objlayout/objio_osd.c
@@ -205,25 +205,36 @@ static void copy_single_comp(struct ore_components *oc, unsigned c,
205int __alloc_objio_seg(unsigned numdevs, gfp_t gfp_flags, 205int __alloc_objio_seg(unsigned numdevs, gfp_t gfp_flags,
206 struct objio_segment **pseg) 206 struct objio_segment **pseg)
207{ 207{
208 struct __alloc_objio_segment { 208/* This is the in memory structure of the objio_segment
209 struct objio_segment olseg; 209 *
210 struct ore_dev *ods[numdevs]; 210 * struct __alloc_objio_segment {
211 struct ore_comp comps[numdevs]; 211 * struct objio_segment olseg;
212 } *aolseg; 212 * struct ore_dev *ods[numdevs];
213 213 * struct ore_comp comps[numdevs];
214 aolseg = kzalloc(sizeof(*aolseg), gfp_flags); 214 * } *aolseg;
215 if (unlikely(!aolseg)) { 215 * NOTE: The code as above compiles and runs perfectly. It is elegant,
216 * type safe and compact. At some Past time Linus has decided he does not
217 * like variable length arrays, For the sake of this principal we uglify
218 * the code as below.
219 */
220 struct objio_segment *lseg;
221 size_t lseg_size = sizeof(*lseg) +
222 numdevs * sizeof(lseg->oc.ods[0]) +
223 numdevs * sizeof(*lseg->oc.comps);
224
225 lseg = kzalloc(lseg_size, gfp_flags);
226 if (unlikely(!lseg)) {
216 dprintk("%s: Faild allocation numdevs=%d size=%zd\n", __func__, 227 dprintk("%s: Faild allocation numdevs=%d size=%zd\n", __func__,
217 numdevs, sizeof(*aolseg)); 228 numdevs, lseg_size);
218 return -ENOMEM; 229 return -ENOMEM;
219 } 230 }
220 231
221 aolseg->olseg.oc.numdevs = numdevs; 232 lseg->oc.numdevs = numdevs;
222 aolseg->olseg.oc.single_comp = EC_MULTPLE_COMPS; 233 lseg->oc.single_comp = EC_MULTPLE_COMPS;
223 aolseg->olseg.oc.comps = aolseg->comps; 234 lseg->oc.ods = (void *)(lseg + 1);
224 aolseg->olseg.oc.ods = aolseg->ods; 235 lseg->oc.comps = (void *)(lseg->oc.ods + numdevs);
225 236
226 *pseg = &aolseg->olseg; 237 *pseg = lseg;
227 return 0; 238 return 0;
228} 239}
229 240