Attention is currently required from: Tim Wawrzynczak, Paul Menzel, Alex Levin, Julius Werner, Jan Dabros. Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62474 )
Change subject: util/cbmem: Add FlameGraph-compatible timestamps output ......................................................................
Patch Set 8:
(6 comments)
File src/commonlib/include/commonlib/timestamp_serialized.h:
https://review.coreboot.org/c/coreboot/+/62474/comment/64870dc7_2635acff PS7, Line 454: uint32_t id_end;
I'd maybe put this in the other table too, as an optional "corresponding end" id for each start id.
It won't work. Look at TS_COPYVPD_START. It has two possible ends. Moreover, I'd like to keep it as separate in order to let compiler/linker strip it from the final coreboot builds, as it probably won't be used by it.
File util/cbmem/cbmem.c:
https://review.coreboot.org/c/coreboot/+/62474/comment/056d6d8f_681f330e PS7, Line 665: process_subrange
Function name should probably still have something to do with timestamps, e.g. […]
Done
https://review.coreboot.org/c/coreboot/+/62474/comment/1b8679a2_d8e93d33 PS7, Line 668: for (uint32_t i = start; i < end;) {
Well... this is... uhh... super complicated. […]
Ack
https://review.coreboot.org/c/coreboot/+/62474/comment/c9698bd6_6d80f352 PS7, Line 715: /* Skip first timestamp, as it only marks the start of coreboot execution */
Don't rely on that, we've recently added the option of inserting "negative" timestamps in front of t […]
Ack
https://review.coreboot.org/c/coreboot/+/62474/comment/fd0fb1f2_22a98bb1 PS7, Line 782: } else {
So I'm basically thinking, rather than using recursion, just maintain an explicit stack of ranges he […]
Wow, that helped reuse a lot of code. Nice :D Btw., I also added end_name (name of end timestamp) to shorten the code a little.
https://review.coreboot.org/c/coreboot/+/62474/comment/0ed28741_e688c8a5 PS7, Line 796: timestamp_print_entry(tse->entry_id, stamp, prev_stamp);
...and then insert the stacked printing here with the others: […]
After few fixes it works well. Thank you. Iterative solution is indeed much better than recursive one :)
-
Jakub Czapiga (Code Review)