Messages in this thread | | | Subject | Re: [PATCH] TRACING: Fix a copmile warning | From | Steven Rostedt <> | Date | Tue, 26 Jul 2011 09:55:52 -0400 |
| |
On Tue, 2011-07-26 at 14:32 +0100, Paulo Marques wrote: > Jesper Juhl wrote:
> >> tb_fmt = kmalloc(sizeof(*tb_fmt), GFP_KERNEL); > >> if (tb_fmt) { > >> fmt = kmalloc(strlen(*iter) + 1, GFP_KERNEL); > >> if (fmt) { > >> list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list); > >> strcpy(fmt, *iter); > >> tb_fmt->fmt = fmt; > >> *iter = tb_fmt->fmt; > >> } else { > >> kfree(tb_fmt); > >> *iter = NULL; > >> } > >> } else { > >> *iter = NULL; > >> } > >> > >> The downside is that the "*iter = NULL" gets repeated twice... > >> > > > > You could avoid that like this: > > > > *iter = NULL; > > tb_fmt = kmalloc(sizeof(*tb_fmt), GFP_KERNEL); > > if (tb_fmt) { > > fmt = kmalloc(strlen(*iter) + 1, GFP_KERNEL); > > if (fmt) { > > list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list); > > strcpy(fmt, *iter); > > tb_fmt->fmt = fmt; > > *iter = tb_fmt->fmt; > > } else { > > kfree(tb_fmt); > > } > > } > > Yes, but this way you always set *iter to NULL, whereas in the previous > version that was the very unlikely case (kmalloc returning NULL). > > Probably gcc is smart enough to generate the same code for both > versions, to avoid setting *iter twice for the likely case (and even if > it doesn't, then the cache will be hot and probably not written back > yet, yadda, yadda)...
gcc may not be allowed to optimize it as iter points to a global, and external functions are called (kmalloc).
But what would work is:
static void hold_module_trace_bprintk_format(const char **start, const char **end) { const char **iter; char *fmt;
mutex_lock(&btrace_mutex); for (iter = start; iter < end; iter++) { struct trace_bprintk_fmt *tb_fmt = lookup_format(*iter); if (tb_fmt) { *iter = tb_fmt->fmt; continue; }
fmt = NULL; tb_fmt = kmalloc(sizeof(*tb_fmt), GFP_KERNEL); if (tb_fmt) { fmt = kmalloc(strlen(*iter) + 1, GFP_KERNEL); if (fmt) { list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list); strcpy(fmt, *iter); tb_fmt->fmt = fmt; } else kfree(tb_fmt); } *iter = fmt; } mutex_unlock(&btrace_mutex); }
The above is easier to read, removes the false warning, and uses local variables that gcc can optimize.
-- Steve
| |