linux-kernel - Re: [PATCH v8 3/5] mm,page_owner: Display all stacks and their count

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ff695557-603c-4bbd-ac6e-3a35ae6ed74f@suse.cz>
Date: Tue, 13 Feb 2024 16:36:01 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Oscar Salvador <osalvador@...e.de>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org, Michal Hocko <mhocko@...e.com>,
 Marco Elver <elver@...gle.com>, Andrey Konovalov <andreyknvl@...il.com>,
 Alexander Potapenko <glider@...gle.com>
Subject: Re: [PATCH v8 3/5] mm,page_owner: Display all stacks and their count

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
>  
>