Hi Kees, my apologies for the (big) delay in answering that! I kept it marked to respond after tests, ended-up forgetting, but now did all the tests and finally I'm able to respond. On 12/10/2022 14:58, Kees Cook wrote: > [...] >> I didn't understand exactly how the mount would override things; I've >> done some tests: >> >> (1) booted with the new kmsg_bytes module parameter set to 64k, and it >> was preserved across multiple mount/umount cycles. >> >> (2) When I manually had "-o kmsg_bytes=16k" set during the mount >> operation, it worked as expected, setting the thing to 16k (and >> reflecting in the module parameter, as observed in /sys/modules). > > What I was imagining was the next step: > > (3) umount, unload the backend, load a new backend, and mount it > without kmsg_bytes specified -- kmsg_bytes will be 16k, not 64k. > > It's a pretty extreme corner-case, I realize. :) However, see below... Oh okay, thanks for pointing that! Indeed, in your test-case I've faced the issue of the retained kmsg_bytes...although, I agree it's an extreme corner-case heheh > [...] > Right, kmsg_bytes is the maximum size to save from the console on a > crash. The design of the ram backend was to handle really small amounts > of persistent RAM -- if a single crash would eat all of it and possibly > wrap around, it could write over useful parts at the end (since it's > written from the end to the front). However, I think somewhere along > the way, stricter logic was added to the ram backend: > > /* > * Explicitly only take the first part of any new crash. > * If our buffer is larger than kmsg_bytes, this can never happen, > * and if our buffer is smaller than kmsg_bytes, we don't want the > * report split across multiple records. > */ > if (record->part != 1) > return -ENOSPC; > > This limits it to just a single record. Indeed, and I already considered that in the past...why was that restricted to a single record, right? I had plans to change it, lemme know your thoughts. (Reference: https://lore.kernel.org/linux-fsdevel/a21201cf-1e5f-fed1-356d-42c83a66fa57@xxxxxxxxxx/) > However, this does _not_ exist for other backends, so they will see up > to kmsg_bytes-size dumps split across psinfo->bufsize many records. For > the backends, this record size is not always fixed: > > - efi uses 1024, even though it allocates 4096 (as was pointed out earlier) > - zone uses kmsg_bytes > - acpi-erst uses some ACPI value from ACPI_ERST_GET_ERROR_LENGTH > - ppc-nvram uses the configured size of nvram partition > > Honestly, it seems like the 64k default is huge, but I don't think it > should be "unlimited" given the behaviors of ppc-nvram, and acpi-erst. > For ram and efi, it's effectively unlimited because of the small bufsizes > (and the "only 1 record" logic in ram). > > Existing documentation I can find online seem to imply making it smaller > (8000 bytes[1], 16000 bytes), but without justification. Even the "main" > documentation[2] doesn't mention it. Right! Also, on top of that, there is a kind of "tricky" logic in which this value is not always respected. For example, in the Steam Deck case we have a region of ~10MB, and set record size of the ramoops backend to 2MB. This is the amount collected, it doesn't respect kmsg_bytes, since it checks the amount dumped vs kmsg_bytes effectively _after_ the first part is written (which in the ramoops case, it's a single write), hence this check is never "respected" there. I don't consider that as a bug, more a flexibility for the ramoops case heh In any way, lemme know if you want to have a "revamp" in the meaning of kmsg_bytes, I'd be glad in discuss/work on that =) Thanks, Guilherme
