Messages in this thread | | | Date | Tue, 08 Nov 2005 10:54:34 -0800 | From | Matthew Dobson <> | Subject | Re: [PATCH 4/8] Cleanup kmem_cache_create() |
| |
Pekka J Enberg wrote: > On Mon, 7 Nov 2005, Matthew Dobson wrote: > >>@@ -1652,9 +1649,9 @@ kmem_cache_t *kmem_cache_create(const ch >> * gfp() funcs are more friendly towards high-order requests, >> * this should be changed. >> */ >>- do { >>- unsigned int break_flag = 0; >>-cal_wastage: >>+ unsigned int break_flag = 0; >>+ >>+ for ( ; ; cachep->gfporder++) { >> cache_estimate(cachep->gfporder, size, align, flags, >> &left_over, &cachep->num); >> if (break_flag) >>@@ -1662,13 +1659,13 @@ cal_wastage: >> if (cachep->gfporder >= MAX_GFP_ORDER) >> break; >> if (!cachep->num) >>- goto next; >>- if (flags & CFLGS_OFF_SLAB && >>- cachep->num > offslab_limit) { >>+ continue; >>+ if ((flags & CFLGS_OFF_SLAB) && >>+ (cachep->num > offslab_limit)) { >> /* This num of objs will cause problems. */ >>- cachep->gfporder--; >>+ cachep->gfporder -= 2; > > > This is not an improvement IMHO. The use of for construct is non-intuitive > and neither is the above. A suggested cleanup is to keep the loop as is but > extract it to a function of its own. > > Pekka
To me the for loop is more readable and intuitive, but that is definitely a matter of opinion. Moving the code to it's own helper function is a better idea than leaving it alone, or changing to a for loop, though. Will resend later today.
Thanks!
-Matt - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |