From: Jani Nikula <jani.nikula@linux.intel.com>
To: Andi Shyti <andi.shyti@linux.intel.com>,
John Harrison <john.c.harrison@intel.com>
Cc: intel-gfx@lists.freedesktop.org, Daniel Vetter <daniel@ffwll.ch>,
dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v5] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
Date: Wed, 06 Sep 2023 13:04:06 +0300 [thread overview]
Message-ID: <87jzt3hb3d.fsf@intel.com> (raw)
In-Reply-To: <ZPhDqTusn9FYY8qV@ashyti-mobl2.lan>
On Wed, 06 Sep 2023, Andi Shyti <andi.shyti@linux.intel.com> 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
next prev parent reply other threads:[~2023-09-06 10:04 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 18:20 [Intel-gfx] [PATCH v5] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset Zhanjun Dong
2023-08-11 19:15 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Avoid circular locking dependency when flush delayed work on gt reset (rev5) Patchwork
2023-08-11 19:15 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-08-11 19:34 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2023-08-21 14:09 ` [Intel-gfx] [PATCH v5] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset Andi Shyti
2023-08-22 13:50 ` Daniel Vetter
2023-08-22 14:14 ` Dong, Zhanjun
2023-08-22 14:28 ` Daniel Vetter
2023-08-22 18:53 ` John Harrison
2023-08-23 16:00 ` Daniel Vetter
2023-08-23 17:37 ` John Harrison
2023-08-28 23:01 ` John Harrison
2023-09-06 6:50 ` Daniel Vetter
2023-09-06 18:40 ` John Harrison
2023-08-31 14:00 ` Andi Shyti
2023-08-31 22:27 ` John Harrison
2023-09-06 9:17 ` Andi Shyti
2023-09-06 10:04 ` Jani Nikula [this message]
2023-09-06 11:02 ` Daniel Vetter
2023-09-06 18:49 ` John Harrison
2023-08-29 10:11 ` Andi Shyti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87jzt3hb3d.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=andi.shyti@linux.intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=john.c.harrison@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox