[PATCH 1/1] tests/qtest/ahci: fix a racy deadlock in the engine stop test

[PATCH 1/1] tests/qtest/ahci: fix a racy deadlock in the engine stop test

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

test_write_engine_stop_in_flight() resumes the suspended backend write
and goes straight on to restart the command engine and read the second
sector back. Whether the write has retired by then is a race. While it
is still outstanding the drive is busy, and handle_cmd() drops a
command that arrives in that state instead of queueing it. Nothing
retries the drop, so the PxCI slot stays set and no D2H FIS is posted.
ahci_command_wait() polls both with no timeout, so the test deadlocks
until the harness kills it.

Drain the port before issuing the read, so the abandoned write has
retired and the engine is idle. Host load stretches the gap between the
resume and the write completing, which is why the deadlock only shows
up on a loaded machine.

Fixes: d05ae87e7a6a ("tests/qtest/ahci: regression test for a PIO write vs. engine stop")
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 tests/qtest/ahci-test.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index b143862ce7..2552fa39f1 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1914,6 +1914,12 @@ static void test_write_engine_stop_in_flight(void)
 
     g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"resume wr\""));
 
+    /*
+     * Retire the abandoned write. handle_cmd() drops a command that arrives
+     * while the drive is still busy and nothing retries it.
+     */
+    g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"aio_flush\""));
+
     /* Round-trip through the device to confirm qemu is still alive. */
     ahci_px_rreg(ahci, port, AHCI_PX_TFD);
 
-- 
2.53.0

Re: [PATCH 1/1] tests/qtest/ahci: fix a racy deadlock in the engine stop test

[Fabiano Rosas]

"Denis V. Lunev" <den@openvz.org> writes:

> From: Denis V. Lunev <den@openvz.org>
>
> test_write_engine_stop_in_flight() resumes the suspended backend write
> and goes straight on to restart the command engine and read the second
> sector back. Whether the write has retired by then is a race. While it
> is still outstanding the drive is busy, and handle_cmd() drops a
> command that arrives in that state instead of queueing it. Nothing
> retries the drop, so the PxCI slot stays set and no D2H FIS is posted.
> ahci_command_wait() polls both with no timeout, so the test deadlocks
> until the harness kills it.
>
> Drain the port before issuing the read, so the abandoned write has
> retired and the engine is idle. Host load stretches the gap between the
> resume and the write completing, which is why the deadlock only shows
> up on a loaded machine.
>
> Fixes: d05ae87e7a6a ("tests/qtest/ahci: regression test for a PIO write vs. engine stop")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  tests/qtest/ahci-test.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index b143862ce7..2552fa39f1 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -1914,6 +1914,12 @@ static void test_write_engine_stop_in_flight(void)
>  
>      g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"resume wr\""));
>  
> +    /*
> +     * Retire the abandoned write. handle_cmd() drops a command that arrives
> +     * while the drive is still busy and nothing retries it.
> +     */
> +    g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"aio_flush\""));
> +
>      /* Round-trip through the device to confirm qemu is still alive. */
>      ahci_px_rreg(ahci, port, AHCI_PX_TFD);

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH 1/1] tests/qtest/ahci: fix a racy deadlock in the engine stop test

[Denis V. Lunev]

On 9/10/26 00:48, Fabiano Rosas wrote:
> "Denis V. Lunev" <den@openvz.org> writes:
>
>> From: Denis V. Lunev <den@openvz.org>
>>
>> test_write_engine_stop_in_flight() resumes the suspended backend write
>> and goes straight on to restart the command engine and read the second
>> sector back. Whether the write has retired by then is a race. While it
>> is still outstanding the drive is busy, and handle_cmd() drops a
>> command that arrives in that state instead of queueing it. Nothing
>> retries the drop, so the PxCI slot stays set and no D2H FIS is posted.
>> ahci_command_wait() polls both with no timeout, so the test deadlocks
>> until the harness kills it.
>>
>> Drain the port before issuing the read, so the abandoned write has
>> retired and the engine is idle. Host load stretches the gap between the
>> resume and the write completing, which is why the deadlock only shows
>> up on a loaded machine.
>>
>> Fixes: d05ae87e7a6a ("tests/qtest/ahci: regression test for a PIO write vs. engine stop")
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> ---
>>  tests/qtest/ahci-test.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
>> index b143862ce7..2552fa39f1 100644
>> --- a/tests/qtest/ahci-test.c
>> +++ b/tests/qtest/ahci-test.c
>> @@ -1914,6 +1914,12 @@ static void test_write_engine_stop_in_flight(void)
>>  
>>      g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"resume wr\""));
>>  
>> +    /*
>> +     * Retire the abandoned write. handle_cmd() drops a command that arrives
>> +     * while the drive is still busy and nothing retries it.
>> +     */
>> +    g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"aio_flush\""));
>> +
>>      /* Round-trip through the device to confirm qemu is still alive. */
>>      ahci_px_rreg(ahci, port, AHCI_PX_TFD);
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
Hi, Fabiano!

Would you mind to get this patch through test tree?
I do not have enough IDE material except this patch
for now.

Thank you in advance,
    Den

Re: [PATCH 1/1] tests/qtest/ahci: fix a racy deadlock in the engine stop test

[Fabiano Rosas]

"Denis V. Lunev" <den@virtuozzo.com> writes:

> On 9/10/26 00:48, Fabiano Rosas wrote:
>> "Denis V. Lunev" <den@openvz.org> writes:
>>
>>> From: Denis V. Lunev <den@openvz.org>
>>>
>>> test_write_engine_stop_in_flight() resumes the suspended backend write
>>> and goes straight on to restart the command engine and read the second
>>> sector back. Whether the write has retired by then is a race. While it
>>> is still outstanding the drive is busy, and handle_cmd() drops a
>>> command that arrives in that state instead of queueing it. Nothing
>>> retries the drop, so the PxCI slot stays set and no D2H FIS is posted.
>>> ahci_command_wait() polls both with no timeout, so the test deadlocks
>>> until the harness kills it.
>>>
>>> Drain the port before issuing the read, so the abandoned write has
>>> retired and the engine is idle. Host load stretches the gap between the
>>> resume and the write completing, which is why the deadlock only shows
>>> up on a loaded machine.
>>>
>>> Fixes: d05ae87e7a6a ("tests/qtest/ahci: regression test for a PIO write vs. engine stop")
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> ---
>>>  tests/qtest/ahci-test.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
>>> index b143862ce7..2552fa39f1 100644
>>> --- a/tests/qtest/ahci-test.c
>>> +++ b/tests/qtest/ahci-test.c
>>> @@ -1914,6 +1914,12 @@ static void test_write_engine_stop_in_flight(void)
>>>  
>>>      g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"resume wr\""));
>>>  
>>> +    /*
>>> +     * Retire the abandoned write. handle_cmd() drops a command that arrives
>>> +     * while the drive is still busy and nothing retries it.
>>> +     */
>>> +    g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"aio_flush\""));
>>> +
>>>      /* Round-trip through the device to confirm qemu is still alive. */
>>>      ahci_px_rreg(ahci, port, AHCI_PX_TFD);
>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Hi, Fabiano!
>
> Would you mind to get this patch through test tree?
> I do not have enough IDE material except this patch
> for now.
>
> Thank you in advance,
>     Den

Yep, no worries. I'm just waiting for my leak fixes series to be ready
and I'll send a PR.