Ben Greear <[email protected]> writes: > On 08/08/2014 01:29 PM, Kalle Valo wrote: >> >> +/* estimated values, hopefully these are enough */ >> +#define ATH10K_ROM_BSS_BUF_LEN 10000 >> +#define ATH10K_RAM_BSS_BUF_LEN 30000 >> + >> /* used for crash-dump storage, protected by data-lock */ >> struct ath10k_fw_crash_data { >> bool crashed_since_read; >> @@ -301,6 +305,9 @@ struct ath10k_fw_crash_data { >> u32 reg_dump_values[REG_DUMP_COUNT_QCA988X]; >> u8 stack_buf[ATH10K_FW_STACK_SIZE]; >> u8 exc_stack_buf[ATH10K_FW_STACK_SIZE]; >> + >> + u8 rom_bss_buf[ATH10K_ROM_BSS_BUF_LEN]; >> + u8 ram_bss_buf[ATH10K_RAM_BSS_BUF_LEN]; >> }; > > That (using estimates instead of allocating memory when we know the > true value and/or when we need it) is wasting quite a bit of RAM. > Doesn't matter on my systems, but AP manufacturers might be more > ticklish about RAM usage...
Yeah, that is a problem bu this can be avoided by disabling CONFIG_ATH10K_DEBUGFS altogether. But on the other hand, there might be people who would still prefer to enable debugfs but not waste RAM on firmware crash dumps. For those we could add a module parameter or similar to disable firmware crash dumps, but is that overengineering already? Another option is that we do firmware crash memory allocation on the fly, when the crash happens. But as we are in atomic context it's pretty expensive. We cannot do huge kmalloc() call due to memory fragmentation and I doubt __vmalloc() is acceptable to call in atomic contexts (at least I didn't find use of that in the kernel). Third option is that we allocate crash dump buffers after the firmware has been loaded and FW IEs are read. That way we don't allocate any extra memory but the fundamental problem still persists. >> + /* These are written to only during first firmware boot so no need >> + * for locking. */ > > It's 'firmware load' instead of boot maybe? Correct, I'll change. -- Kalle Valo _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
