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