Re: [PATCH v2 4/4] selftests/mm: Fix test result reporting in gup_longterm

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

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.