On 2/13/24 16:33, Oscar Salvador wrote:
> On Tue, Feb 13, 2024 at 03:25:26PM +0100, Vlastimil Babka wrote:
>> On 2/12/24 23:30, Oscar Salvador wrote:
>> > +static int stack_print(struct seq_file *m, void *v)
>> > +{
>> > + char *buf;
>> > + int ret = 0;
>> > + struct stack *stack = v;
>> > + struct stack_record *stack_record = stack->stack_record;
>> > +
>> > + if (!stack_record->size || stack_record->size < 0 ||
>> > + refcount_read(&stack_record->count) < 2)
>> > + return 0;
>> > +
>> > + buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>> > +
>> > + ret += stack_trace_snprint(buf, PAGE_SIZE, stack_record->entries,
>> > + stack_record->size, 0);
>> > + if (!ret)
>> > + goto out;
>> > +
>> > + scnprintf(buf + ret, PAGE_SIZE - ret, "stack_count: %d\n\n",
>> > + refcount_read(&stack_record->count));
>> > +
>> > + seq_printf(m, buf);
>> > + seq_puts(m, "\n\n");
>> > +out:
>> > + kfree(buf);
>>
>> Seems rather wasteful to do kzalloc/kfree so you can print into that buffer
>> first and then print/copy it again using seq_printf. If you give up on using
>> stack_trace_snprintf() it's not much harder to print the stack directly with
>> a loop of seq_printf. See e.g. slab_debugfs_show().
>
> Well, I thought about not reinventing the wheel there, but fair enough
> than performing a kmalloc/free op on every print might be suboptimal.
> I will try to do ir with seq_printf alone.
Of course once there's more than one stackdepot user printing into a
seq_file, creating a common seq_file helper analogy of
stack_trace_snprintf() and using it from all places, would also be an option :)
> Thanks
>
>
