Re: [PATCH v7 08/17] unwind_user/deferred: Add unwind cache

[Ingo Molnar]

* Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> -struct unwind_task_info {
> +struct unwind_cache {
>  	unsigned long		*entries;
> +	unsigned int		nr_entries;
> +};
> +
> +struct unwind_task_info {
> +	struct unwind_cache	cache;
>  };

> @@ -19,17 +20,29 @@ int unwind_deferred_trace(struct unwind_stacktrace *trace)
>  	if (current->flags & PF_EXITING)
>  		return -EINVAL;
>  
> -       if (!info->entries) {
> -               info->entries = kmalloc_array(UNWIND_MAX_ENTRIES, sizeof(long),
> -                                             GFP_KERNEL);
> -               if (!info->entries)
> -		       return -ENOMEM;
> +	if (!cache->entries) {
> +		cache->entries = kmalloc_array(UNWIND_MAX_ENTRIES, sizeof(long),
> +					       GFP_KERNEL);
> +		if (!cache->entries)
> +			return -ENOMEM;
> +        }

That's just sloppy: why isn't the unwind_cache a pointer to begin with, 
with the dynamically allocated object containing ::nr_entries?

Also, the code has whitespace damage.

> +
> +	trace->entries = cache->entries;
> +
> +	if (cache->nr_entries) {
> +               /*
> +                * The user stack has already been previously unwound in this
> +                * entry context.  Skip the unwind and use the cache.
> +                */
> +               trace->nr = cache->nr_entries;
> +               return 0;

Whitespace damage here too. Maybe in other patches as well.

Please don't rush this series without first reviewing it carefully ...

Thanks,

	Ingo