On Wed, 06 Sep 2023, Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> wrote: >> > I was actually thinking why not leave things as they are and just >> > disable lockdep from CI. This doesn't look like a relevant report >> > to me. >> > >> > Andi >> Disable lockdep? The whole of lockdep? We absolutely do not want to disable >> an extremely important deadlock testing infrastructure in our test >> framework. That would be defeating the whole point of CI. >> >> Potentially we could annotate this one particular scenario to suppress this >> one particular error. But it seems simpler and safer to just update the >> code to not hit that scenario in the first place. > > yes... lockdep is a debug tool and might provide false reports... > We need to have a great willingness to start fixing and hunting > debug lockdep's false positives (like this one, for instance). > > It's even more annoying to reduce our CI pass rates, especially > when in BAT tests, with such false deadlocks. Make lockdep understand what you're doing, and there are no false positives. That's all there is to it. > It's the developer's responsibility to test its code with > debug_lockdep and fix all the potential deadlocks and ignore the > false ones. No. Manual validation of lockdep reports is not feasible. Lockdep is the tool to validate locking. It's the developer's responsibility to make lockdep understand the design. Besides, locking is often subtle. Stuff can change as a side effect even when you're not intentionally changing locking, e.g. during refactoring. What you're suggesting effectively means all developers should run all of igt on a bunch of different generations of machines with lockdep enabled. Realistically, not going to happen, and we have CI because of this. > I sent a patch for this[*] already. > > Andi > > [*] https://gitlab.freedesktop.org/gfx-ci/i915-infra/-/merge_requests/128 Yeah, no. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
