Mark, I'm not finding this productive. Bottom line is you've broken the tests, please fix them or if you're not willing to I'll send a fix.
Thanks. On Thu, Jun 05, 2025 at 06:38:36PM +0100, Mark Brown wrote: > On Thu, Jun 05, 2025 at 06:09:09PM +0100, Lorenzo Stoakes wrote: > > On Thu, Jun 05, 2025 at 05:42:55PM +0100, Mark Brown wrote: > > > > > Better to do all of these formating fixes and maintain the _same > > > > behaviour_ then > > > > separately tackle whether or not we should skip. > > > > I'm confused, that's generally the opposite of the standard advice for > > > the kernel - usually it's fixes first, then deal with anything cosmetic > > > or new? > > > I mean the crux is that the 'cosmetic' changes also included a 'this might > > break things' change. > > No, the cosmetic changes are separate. I'm just saying I have a small > bunch of stuff based on David's feedback to send out after the merge > window. > > > I'm saying do the cosmetic things in _isolation_, or fix the brokenness > > before doing the whole lot. > > Some subsystems will complain if you send anything that isn't urgent > during the merge window, this looked more like an "I suppose you could > configure the kernel that way" problem than a "people will routinely run > into this" one, I was expecting it (or something) to go in as a fix but > that it was safer to wait for -rc1 to send. > > > > > Obviously the better option would be to somehow determine if hugetlb is > > > > available in advance (of course, theoretically somebody could come in > > > > and > > > > reserve pages but that's not veyr likely). > > > > The tests do enumerate the set of available hugepage sizes at runtime > > > (see the loop in run_test_case()) but detect_hugetlb_page_sizes() just > > > looks in /sys/kernel/mm/hugepages/ for subdirectories and doesn't look > > > inside those directories to see if there are actually any huge pages > > > available for the huge page sizes advertised. There's probably utility > > > in at least a version of that function that checks. > > > Right yes, I mean obviously this whole thing is a mess already that's not > > your fault, and ideally we'd have some general way of looking this up > > across _all_ tests and just switch things on/off accordingly. > > That is at least library code so it'd get the three tests that use it, > though possibly one of them actually wants the current behaviour for > some reason? > > > There's a whole Pandora's box about what the tests should assume/not and > > yeah. Anyway. Maybe leave it closed for now :) > > It's separate, yeah. It'd also be good to document what you need to > enable all the tests somewhere as well - there's the config fragment > already which is good, but you also at least need a bunch of command > line options to set up huge pages and enable secretmem.
