|
| 1 | +Submitting patches to bcachefs: |
| 2 | +=============================== |
| 3 | + |
| 4 | +Patches must be tested before being submitted, either with the xfstests suite |
| 5 | +[0], or the full bcachefs test suite in ktest [1], depending on what's being |
| 6 | +touched. Note that ktest wraps xfstests and will be an easier method to running |
| 7 | +it for most users; it includes single-command wrappers for all the mainstream |
| 8 | +in-kernel local filesystems. |
| 9 | + |
| 10 | +Patches will undergo more testing after being merged (including |
| 11 | +lockdep/kasan/preempt/etc. variants), these are not generally required to be |
| 12 | +run by the submitter - but do put some thought into what you're changing and |
| 13 | +which tests might be relevant, e.g. are you dealing with tricky memory layout |
| 14 | +work? kasan, are you doing locking work? then lockdep; and ktest includes |
| 15 | +single-command variants for the debug build types you'll most likely need. |
| 16 | + |
| 17 | +The exception to this rule is incomplete WIP/RFC patches: if you're working on |
| 18 | +something nontrivial, it's encouraged to send out a WIP patch to let people |
| 19 | +know what you're doing and make sure you're on the right track. Just make sure |
| 20 | +it includes a brief note as to what's done and what's incomplete, to avoid |
| 21 | +confusion. |
| 22 | + |
| 23 | +Rigorous checkpatch.pl adherence is not required (many of its warnings are |
| 24 | +considered out of date), but try not to deviate too much without reason. |
| 25 | + |
| 26 | +Focus on writing code that reads well and is organized well; code should be |
| 27 | +aesthetically pleasing. |
| 28 | + |
| 29 | +CI: |
| 30 | +=== |
| 31 | + |
| 32 | +Instead of running your tests locally, when running the full test suite it's |
| 33 | +prefereable to let a server farm do it in parallel, and then have the results |
| 34 | +in a nice test dashboard (which can tell you which failures are new, and |
| 35 | +presents results in a git log view, avoiding the need for most bisecting). |
| 36 | + |
| 37 | +That exists [2], and community members may request an account. If you work for |
| 38 | +a big tech company, you'll need to help out with server costs to get access - |
| 39 | +but the CI is not restricted to running bcachefs tests: it runs any ktest test |
| 40 | +(which generally makes it easy to wrap other tests that can run in qemu). |
| 41 | + |
| 42 | +Other things to think about: |
| 43 | +============================ |
| 44 | + |
| 45 | +- How will we debug this code? Is there sufficient introspection to diagnose |
| 46 | + when something starts acting wonky on a user machine? |
| 47 | + |
| 48 | + We don't necessarily need every single field of every data structure visible |
| 49 | + with introspection, but having the important fields of all the core data |
| 50 | + types wired up makes debugging drastically easier - a bit of thoughtful |
| 51 | + foresight greatly reduces the need to have people build custom kernels with |
| 52 | + debug patches. |
| 53 | + |
| 54 | + More broadly, think about all the debug tooling that might be needed. |
| 55 | + |
| 56 | +- Does it make the codebase more or less of a mess? Can we also try to do some |
| 57 | + organizing, too? |
| 58 | + |
| 59 | +- Do new tests need to be written? New assertions? How do we know and verify |
| 60 | + that the code is correct, and what happens if something goes wrong? |
| 61 | + |
| 62 | + We don't yet have automated code coverage analysis or easy fault injection - |
| 63 | + but for now, pretend we did and ask what they might tell us. |
| 64 | + |
| 65 | + Assertions are hugely important, given that we don't yet have a systems |
| 66 | + language that can do ergonomic embedded correctness proofs. Hitting an assert |
| 67 | + in testing is much better than wandering off into undefined behaviour la-la |
| 68 | + land - use them. Use them judiciously, and not as a replacement for proper |
| 69 | + error handling, but use them. |
| 70 | + |
| 71 | +- Does it need to be performance tested? Should we add new peformance counters? |
| 72 | + |
| 73 | + bcachefs has a set of persistent runtime counters which can be viewed with |
| 74 | + the 'bcachefs fs top' command; this should give users a basic idea of what |
| 75 | + their filesystem is currently doing. If you're doing a new feature or looking |
| 76 | + at old code, think if anything should be added. |
| 77 | + |
| 78 | +- If it's a new on disk format feature - have upgrades and downgrades been |
| 79 | + tested? (Automated tests exists but aren't in the CI, due to the hassle of |
| 80 | + disk image management; coordinate to have them run.) |
| 81 | + |
| 82 | +Mailing list, IRC: |
| 83 | +================== |
| 84 | + |
| 85 | +Patches should hit the list [3], but much discussion and code review happens on |
| 86 | +IRC as well [4]; many people appreciate the more conversational approach and |
| 87 | +quicker feedback. |
| 88 | + |
| 89 | +Additionally, we have a lively user community doing excellent QA work, which |
| 90 | +exists primarily on IRC. Please make use of that resource; user feedback is |
| 91 | +important for any nontrivial feature, and documenting it in commit messages |
| 92 | +would be a good idea. |
| 93 | + |
| 94 | +[0]: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git |
| 95 | +[1]: https://evilpiepirate.org/git/ktest.git/ |
| 96 | +[2]: https://evilpiepirate.org/~testdashboard/ci/ |
| 97 | + |
| 98 | +[4]: irc.oftc.net#bcache, #bcachefs-dev |
0 commit comments