Add the lock ordering blueprint, and teach bpc to draw what is not a trace - #68
Merged
Merged
Conversation
…trace The third blueprint the compiler generates sections 2, 5 and 7 for, which is the M1 exit criterion moving from two to three. The document is the lock ordering checker on the pinned 7.2.2 kernel, i386, built with the D-lockdep profile. Fifty nine citations, all confirmed against the unpacked tree, all resolving to exactly one line in the file they name. Sections 2 and 7 come out of that build's BTF and section 5 comes out of three artefacts taken on one boot of it. Section 5 could not have been written before this change. It only knew how to draw a function_graph capture, so every other kind of artefact got a single line reading "Tracer `unknown`, recording ...". All three lockdep artefacts are non-trace, so the section would have been three shrugs. Now a capture nobody traced says how it was taken, using the command its own metadata carries, and two readers draw the lockdep report and the /proc counters properly. Which reader draws a file is decided by the routing table in kxray/corpus/index.py rather than by a new key in the metadata, so a parser and the blueprints that quote it cannot disagree about what a file is. The counters reader prints all fifty one rows rather than a chosen handful, because a generator that picks the interesting ones is a generator with an opinion, and an opinion in a generated section is what the seal exists to keep out. The two /proc readings are drawn as one table with a change column. They came off one boot on purpose, and the claim is about what moved, so fifty numbers in one column and fifty in another pages apart would not have said it. Which two go together comes off the `pair` key each capture already carries, and which is the before is the order the blueprint lists them in, so nothing guesses from a filename. Six of the functions the blueprint names have no symbol in the build, because the compiler inlined them. Section 7 says so in its table, and section 8 says why that is a different fact from a function the configuration removed, even though the two look identical from outside. The ledger gains a scheduler entry, because the held lock array lives in include/linux/sched.h and something now reaches into it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The third blueprint the compiler generates sections 2, 5 and 7 for. The M1 exit criterion asks for five and there were two, so this moves it to three.
What the blueprint is
The lock ordering checker, on the pinned 7.2.2 kernel, i386, built with the
D-lockdepprofile. Fifty nine citations, every one confirmed against the unpacked tree, every anchor matching exactly one line in the file it names. Sections 2 and 7 come out of that build's BTF. Section 5 comes out of three artefacts taken on a single boot of it.It leads with the thing readers get wrong, which is that this mechanism does not watch for a deadlock. It keeps a graph of lock classes, and every time it is about to add an edge it asks whether the edge closes a cycle. That question is asked during an acquire, on one processor, with nothing waiting for anything, which is why a report can come out of a run that finished fine on a machine with one processor. The pinned machine has one processor, so that is not a footnote here, it is the reason any of this is demonstrable at all.
The second thing it leads with is the part that costs people an afternoon. The first report of a boot switches the whole mechanism off, and the only place that is written down is one line of a
/procfile. The two/proc/lockdep_statsreadings in the corpus are before and after exactly that, and section 5 now puts them side by side.Status is
partial, which means the same as it does on the other two. Everything mechanical is done and a person has not read it.Why bpc had to change first
Section 5 only knew how to draw a
function_graphcapture. Every other kind of artefact got one line readingTracer `unknown`, recording .... All three lockdep artefacts are non-trace, so the section would have been three shrugs and the blueprint would have had no observable behaviour in it worth reading.Three changes:
Not a trace. Taken by 'insmod /lib/modules/abba.ko'is a true and useful sentence whereTracer 'unknown'was neither./proccounters. The report reader prints the task, the cycle, the two locks with their usage and wait types, the numbered chain, the interleaving the kernel says would deadlock, and the stack behind each link, all rebuilt from the parse rather than copied out of the file. Printing it back in the shape the parser understood it is the section, and it doubles as proof that the shape is understood rather than the words being quoted.Two design points that were deliberate rather than convenient.
Which reader draws a file is decided by the routing table in
kxray/corpus/index.py, not by a new key in the metadata. That table already exists becausetools/baselineaccounts every line of every artefact through it. A second registry would be a second thing to keep in step, and the failure mode of it drifting is a parser and a blueprint disagreeing about what a file is.The counters reader prints all fifty one rows rather than a chosen handful. A generator that picks the interesting rows is a generator with an opinion, and an opinion inside a generated section is the thing the seal exists to keep out. The change column makes the rows that moved findable without anything in the tool deciding which they are.
The pairing takes the
pairkey each capture already carries, because being half of a pair is a fact about how it was taken. Which of the two is the before is the order the blueprint lists them in, so the author decides it and nothing guesses from a word in a filename.The six functions that are not there
validate_chain,check_prev_add,check_prevs_add,lookup_chain_cache,add_chain_cacheanditerate_chain_keyhave no symbol in the build. They arestaticwith one caller in a file the compiler has room in, and it inlined them. Section 7 printsno symbol in this build, inlined or configured outfor each, which is a true fact about the build rather than a gap in the document.Section 8 says why that is a different fact from a function the configuration removed. A symbol missing because the code is not there means the work does not happen. A symbol missing because the compiler inlined it means the work happens and cannot be observed. They look identical from outside and only the build can tell them apart, which is the whole argument for generating section 7 from the kernel rather than typing a list of names.
The rest
coverage.tomlgains aschedulerentry.MAX_LOCK_DEPTHand the held lock array live ininclude/linux/sched.h, so something now reaches into the scheduler and the ledger has to say so. Thelockingentry picks up the two lockdep headers and names this blueprint.Three new tests in
tests/test_bpc.pyfor the non-trace path, the report rendering and the paired table. The count intest_show_groups_by_status_and_countsis now read off the ledger rather than typed in, because it moves every time a blueprint reaches into a subsystem nobody had written about yet, and a test that has to be edited for that is a test that gets edited without being read.There is a picture, built by
tools.diagramsinto both SVG and excalidraw from one source file, showing the graph with the edge that closes the cycle, the two threads that never waited, the four things done on every acquire, and the band across the bottom fordebug_locksgoing to zero.Gate
Ruff, ruff format, pytest, the node tests, and every tool check.
bpc: 4 blueprint(s) clean,refcheck: paths clean, 132 citation(s),baseline: 41 artefact(s), 7968 line(s), 0 unparsed,coverage: 31 subsystem(s), 0 taught, 8 partial, 19 mentioned, 4 out-of-scope.Part of #2.