On Wed, Sep 09 2015, Rasmus Villemoes <li...@rasmusvillemoes.dk> wrote:
ping
> The kernel now has kstrdup_const/kfree_const for reusing .rodata
> (typically string literals) when possible; there's no reason to
> duplicate that logic in the tracing system. Moreover, as the comment
> above core_kernel_data states, it may not always return true for
> .rodata - that is for example the case on x86_64, where we thus end up
> kstrdup'ing all the passed-in strings.
>
> Arguably, testing for .rodata explicitly (as kstrdup_const does) is
> also more correct: I don't think one is supposed to be able to change
> the name after creating the event_subsystem by passing the address of
> a static char (but non-const) array.
>
> Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
> ---
> kernel/trace/trace_events.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 7ca09cdc20c2..ae97d73b31fa 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -38,21 +38,19 @@ static LIST_HEAD(ftrace_common_fields);
> static struct kmem_cache *field_cachep;
> static struct kmem_cache *file_cachep;
>
> -#define SYSTEM_FL_FREE_NAME (1 << 31)
> -
> static inline int system_refcount(struct event_subsystem *system)
> {
> - return system->ref_count & ~SYSTEM_FL_FREE_NAME;
> + return system->ref_count;
> }
>
> static int system_refcount_inc(struct event_subsystem *system)
> {
> - return (system->ref_count++) & ~SYSTEM_FL_FREE_NAME;
> + return system->ref_count++;
> }
>
> static int system_refcount_dec(struct event_subsystem *system)
> {
> - return (--system->ref_count) & ~SYSTEM_FL_FREE_NAME;
> + return --system->ref_count;
> }
>
> /* Double loops, do not use break, only goto's work */
> @@ -460,8 +458,7 @@ static void __put_system(struct event_subsystem *system)
> kfree(filter->filter_string);
> kfree(filter);
> }
> - if (system->ref_count & SYSTEM_FL_FREE_NAME)
> - kfree(system->name);
> + kfree_const(system->name);
> kfree(system);
> }
>
> @@ -1492,13 +1489,9 @@ create_new_subsystem(const char *name)
> system->ref_count = 1;
>
> /* Only allocate if dynamic (kprobes and modules) */
> - if (!core_kernel_data((unsigned long)name)) {
> - system->ref_count |= SYSTEM_FL_FREE_NAME;
> - system->name = kstrdup(name, GFP_KERNEL);
> - if (!system->name)
> - goto out_free;
> - } else
> - system->name = name;
> + system->name = kstrdup_const(name, GFP_KERNEL);
> + if (!system->name)
> + goto out_free;
>
> system->filter = NULL;
>
> @@ -1511,8 +1504,7 @@ create_new_subsystem(const char *name)
> return system;
>
> out_free:
> - if (system->ref_count & SYSTEM_FL_FREE_NAME)
> - kfree(system->name);
> + kfree_const(system->name);
> kfree(system);
> return NULL;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
