Summary: | [CI][BAT] igt@debugfs_test@read_all_entries* - incomplete | ||
---|---|---|---|
Product: | DRI | Reporter: | Martin Peres <martin.peres> |
Component: | DRM/Intel | Assignee: | Intel GFX Bugs mailing list <intel-gfx-bugs> |
Status: | CLOSED NOTABUG | QA Contact: | Intel GFX Bugs mailing list <intel-gfx-bugs> |
Severity: | normal | ||
Priority: | highest | CC: | intel-gfx-bugs, martin.peres, mika.kahola, stanislav.lisovskiy |
Version: | XOrg git | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | ReadyForDev | ||
i915 platform: | HSW | i915 features: |
Description
Martin Peres
2018-12-20 08:52:00 UTC
Patch to spin forever on while (crc.entries), spins forever on while (crc.entries). Quelle surprise? (In reply to Chris Wilson from comment #1) > Patch to spin forever on while (crc.entries), spins forever on while > (crc.entries). Quelle surprise? I guess you mean https://patchwork.freedesktop.org/patch/268758/ which is from Mika Kahola: + while (crtc->crc.entries) { + usleep_range(100, 200); + } + But my patch doesn't have any while loops, however it also fails with the same test case, see https://patchwork.freedesktop.org/series/54267/. Or then it might mean, that we both managed somehow to introduce same regression. Yet you have the similar assumption about the state of crc.entries and a corresponding lockup during crc collection. Circumstantial, maybe. More likely that the assumption on crc.entries is incorrect and you should try and capture your knowledge of crc.entries and how the driver may differ in an igt. (In reply to Chris Wilson from comment #3) > Yet you have the similar assumption about the state of crc.entries and a > corresponding lockup during crc collection. Circumstantial, maybe. More > likely that the assumption on crc.entries is incorrect and you should try > and capture your knowledge of crc.entries and how the driver may differ in > an igt. What do you mean by "similar lockup"? I guess previously you were talking about while loop in Mika's patch, waiting on crc.entries, with probably infinite loop, however I'm not doing that, there is only one check for crc.entries state, ok this could be also wrong, however in other code parts this is done exactly same way, that is what I used as example: int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, uint32_t frame, uint32_t *crcs) { struct drm_crtc_crc *crc = &crtc->crc; struct drm_crtc_crc_entry *entry; int head, tail; spin_lock(&crc->lock); /* Caller may not have noticed yet that userspace has stopped reading */ if (!crc->entries) { spin_unlock(&crc->lock); return -EINVAL; } ... My patch was just a quick experiment. It was never meant to be a complete solution. I wanted to try out if we need to wait some time if we have crc entries. As I didn't know how much waiting time would have been enough, I tried this idea with a possibly infinite loop. That's why I tried out this idea on trybot and never sent the patch in mailing list for review. It turned out in Trybot run that the idea didn't really work. I checked out in intel-gfx and found no patches hit by this issue. So it indeed is likely a regression found in your patches. Sorry for the noise. Closing! I just run these debugfs_tests, with my patch on top of drm-tip on my local ICL U - it doesn't fail even after few runs.. Which is weird :) |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.