On Tue, Apr 16, 2019 at 1:03 AM Peter Zijlstra <pet...@infradead.org> wrote: > > Using 320 bytes of stack space for a 40 byte structure is ludicrous and > clearly not right.
Ack. That said, I wish we didn't have these stack structures at all. Or at least were more careful about them. For example, another case of this struct on the stack looks really iffy too: struct flush_tlb_info info; info.start = start; info.end = end; on_each_cpu(do_kernel_range_flush, &info, 1); note how it only initializes two of the fields, and leaves the others entirely randomly initialized with garbage? Yeah, yeah, "do_kernel_range_flush()" only uses those two fields, but it still makes my skin crawl how we basically pass a largely uninitialized structure and have other CPU's look at it. And in another case we do have a nicely initialized structure void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch) { struct flush_tlb_info info = { .mm = NULL, .start = 0UL, .end = TLB_FLUSH_ALL, }; but it looks like it shouldn't have been on the stack in the first place, because as far as I can tell it's entirely constant, and it should just be a "static const" structure initialized at compile time. So as far as I can tell, we could do something like -static void flush_tlb_func_local(void *info, enum tlb_flush_reason reason) +static void flush_tlb_func_local(const void *info, enum tlb_flush_reason reason) - struct flush_tlb_info info = { + static const struct flush_tlb_info info = { for that case. End result: it looks like we have three of these stack things, and all three had something odd in them. So very much Ack on that patch, but maybe we could do a bit more cleanup here? Linus