Re: [PATCH v1] dmaengine: dmatest: Fix dmatest waiting less when interrupted

[Nathan Lynch <nathan.lynch@amd.com>]

Hi Vinicius,

Vinicius Costa Gomes <vinicius.gomes@intel.com> writes:
> Nathan Lynch <nathan.lynch@amd.com> writes:
>> Vinicius Costa Gomes <vinicius.gomes@intel.com> writes:
>>> Change the "wait for operation finish" logic to take interrupts into
>>> account.
>>>
>>> When using dmatest with idxd DMA engine, it's possible that during
>>> longer tests, the interrupt notifying the finish of an operation
>>> happens during wait_event_freezable_timeout(), which causes dmatest to
>>> cleanup all the resources, some of which might still be in use.
>>>
>>> This fix ensures that the wait logic correctly handles interrupts,
>>> preventing premature cleanup of resources.
>>>
>>> Reported-by: kernel test robot <oliver.sang@intel.com>
>>> Closes: https://lore.kernel.org/oe-lkp/202502171134.8c403348-lkp@intel.com
>>
>> Given the report at the URL above I'm struggling to follow the rationale
>> for this change. It looks like a use-after-free in idxd while
>> resetting/unbinding the device, and I can't see how changing whether
>> dmatest threads perform freezeable waits would change this.
>>
>
> I think that the short version is that the reproducition script triggers
> different problems on different platforms/configurations.
>
> The solution I proposed fixes a problem I was seeing on a SPR system, on
> a GNR system (that I was only able to get later) I see something more similar
> to this particular splat (currently working on the fix).
>
> Seeing this question, I realize that I should have added a note to the
> commit detailing this.
>
> So I am planning on proposing two (this and another) fixes for the same
> report, hoping that it's not that confusing/unusual.

I'm still confused... why is wait_event_freezable_timeout() the wrong
API for dmatest to use, and how could changing it to
wait_event_timeout() cause it to "take interrupts into account" that it
didn't before?

AFAIK the only change made here is that dmatest threads effectively
become unfreezeable, which is contrary to prior authors' intentions:

commit 981ed70d8e4f ("dmatest: make dmatest threads freezable")
commit adfa543e7314 ("dmatest: don't use set_freezable_with_signal()")