On Tue, Sep 24, 2024 at 08:09:38AM -0700, Guenter Roeck wrote: > On 9/24/24 06:56, Maxime Ripard wrote: > > On Tue, Sep 24, 2024 at 06:37:59AM GMT, Guenter Roeck wrote: > > > On 9/24/24 04:54, Maxime Ripard wrote: > > > > +Guenter > > > > > > > > On Tue, Sep 24, 2024 at 12:06:28PM GMT, Simona Vetter wrote: > > > > > On Tue, Sep 17, 2024 at 08:43:50PM +0300, Jani Nikula wrote: > > > > > > The tests consistently trigger WARNs in drm_framebuffer code. I'm not > > > > > > sure what the point is with type of belts and suspenders tests. The > > > > > > warnings *are* the way to flag erroneous API usage. > > > > > > > > > > > > Warnings in turn trigger failures in CI. Filtering the warnings are > > > > > > error prone, and, crucially, would also filter actual errors in case the > > > > > > kunit tests are not run. > > > > > > > > > > > > I acknowledge there may be complex test cases where you'd end up > > > > > > triggering warnings somewhere deep, but these are not it. These are > > > > > > simple. > > > > > > > > > > > > Revert the tests, back to the drawing board. > > > > > > > > > > Yeah I think long-term we might want a kunit framework so that we can > > > > > catch dmesg warnings we expect and test for those, without those warnings > > > > > actually going to dmesg. Similar to how the lockdep tests also reroute > > > > > locking validation, so that the expected positive tests don't wreak > > > > > lockdep for real. > > > > > > > > > > But until that exists, we can't have tests that splat in dmesg when they > > > > > work as intended. > > > > > > FWIW, that is arguable. More and more tests are added which do add such splats, > and I don't see any hesitance by developers to adding more. So far I counted > two alone in this commit window, and that does not include new splats from > tests which I had already disabled. I simply disable those tests or don't > enable them in the first place if they are new. I did the same with the drm > unit tests due to the splats generated by the scaling unit tests, so any > additional drm unit test splats don't make a difference for me since the > tests are already disabled. I think for at least drm the consensus is clear, we won't have kunit tests that splat. Personally I really don't see the point of unit tests that are somewhere between unecessarily hard or outright too much pain to deploy in a test rig: Either you don't run them (not great), or you filter splats and might filter too much (not great either) or you filter as little as possible and fight false positives (also kinda suboptimal). Especially for unit tests this stuff must be totally rock solid. We've had similar discussions in the past around unit tests that wasted too much cpu time with randomized combinatorial testing, and we've thrown those out too from drm. Test hygiene matters. Also this means if you see this stuff in drm unit tests (or related things like dma-buf/fence), please report or send revert. > > > > It should be pretty soon: > > > > https://lore.kernel.org/dri-devel/20240403131936.787234-1-linux@xxxxxxxxxxxx/ > > > > > > > > I'm not sure what happened to that series, but it looks like everybody > > > > was mostly happy with it? > > > > > > > > > > Major subsystem maintainers did not provide any feedback at all, and then > > > there came the "it is not perfect enough" feedback, so I gave up on it. > > > > Well, if that means anything, we're interested even in something > > imperfect if it solves the above case :) > > > > Maybe someone else is interested picking it up (and no need for credits). > I am buried in work and don't have the time (nor, frankly, much interest) > to revive it. Also, just for reference: The patch series touches a total > of 8 architectures, and unless I missed some I only got feedback from two > architecture maintainers. That doesn't include arm - I don't recall if it > doesn't need changes or if I never got there. This is great, somehow I missed it and much appreciated for the initial version even if you can't push it further. Hopefully one of the folks working on drm kunit tests will pick it up, it will be needed here. Cheers, Sima -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
