On Fri, Feb 07, 2025 at 10:50:05PM +0100, Andreas Gruenbacher wrote: > On Fri, Feb 7, 2025 at 7:56 PM Kent Overstreet > <kent.overstreet@xxxxxxxxx> wrote: > > On Tue, Jan 28, 2025 at 05:38:37PM +0100, Andreas Gruenbacher wrote: > > > Kent, to continue our discussion from last November, I've gone through > > > more parts of the eytzinger code and as a result, here are some patches > > > for you to consider. > > > > > > What I've not looked at are the eytzinger_to_inorder and > > > inorder_to_eytzinger functions, as well as the implementation of sort. > > > Those functions could use a bit more documentation, but the code iself > > > looks reasonable. > > > > > > > > > Shuah, I've also had a quick look at converting the tests into kernel > > > selftests, but that hasn't gone very far because of the lack of support > > > for basic functions like __fls(), __ffs(), ffz(), and > > > rounddown_pow_of_two() in selftests. Are there any plans for making > > > those kinds of primitives generally available to selftests? > > > > Ping on this patchset - can you throw up a git repo that's ready to be > > applied directly, and have you run the test that failed yourself to > > confirm the fix? > > Did you miss this message? > > https://lore.kernel.org/linux-bcachefs/20250130103400.1899121-1-agruenba@xxxxxxxxxx/ No, I saw it. I'm looking for a branch that is ready to be applied as is - fixups in the correct place, no debug patches. That way there's no confusion over exactly what is going in, and we elimanate a lot of the back and forth. > > The test results at: > > https://evilpiepirate.org/~testdashboard/ci?user=kmo&branch=eytzinger > > are still for the initial version, but I've applied the fix right away: > > https://git.kernel.org/pub/scm/linux/kernel/git/agruen/linux.git/log/?h=bcachefs I shall point my CI at that... > On top of that, I've got a small patch that adds eytzinger0_find self > tests. That patch still needs testing before I can post it, though. > > Oh, and let me warn you about "bcachefs: Run the eytzinger tests on > modprobe" again: this really is FOR DEBUGGING ONLY, so please do not > merge. Why is that in the branch? You ran the eytzinger tests, and that's for code that's self contained that I'm not concerned about running it myself (and if I did, then we'd be following up on getting it working with kunit now...) If you want to leave it in the branch as an informational thing ("yes these tests were run and here's how"), it should be at the top, not the bottom, so that when I take it out I still have a subset that's an exact match (to the sha1) of something you've been working with and testing.
