On Wed, 21 Oct 2026 at 06:36, Rodrigo Vivi <[email protected]> wrote:
>
> The new commit 'drm/i915/gem: Don't fault on presumed_offset write-back' [1]
>
> reverts commit '97a37c919f62 ("drm/i915/gem: Ignore readonly failures when
> updating relocs")'
>
> And this blows up exec_reloc concurrent tests that uses
> mprotect(reloc, sz, PROT_READ); [2]
The old code had two separate bugs:
- it used __put_user() while holding locks that could deadlock with
fault handling
Fixed with the pagefault_disable() around the put_user()
- it didn't actually check failures, so a read-only area silently
just dropped the write
Fixed with that "if (unlikely(err))" addition
But yes, I didn't see that the error handling had been removed - the
removal didn't show up in the code
So I'll remove the error handling from me branch before merging.
But that code absolutely smells, though. It makes no sense to silently
drop the relocations and just go on - and continue in that loop that
will then drop the next one. And the one after.. You're just wasting
CPU time and doing nothing useful.
So I will merge it with that error handling removed, but I suspect
that test is bogus garbage, and was testing for a bug, not testing for
actual behavior. But I may be wrong.
But the __put_user() and the pagefault_disable() around it stays. The
old code was very very buggy - and not in a "waste CPU time" way, but
in a "you could cause deadlocks and lockdep issues" kind of way.
Linus
