Re: [PATCH] Stop all RingWatchdog threads

[Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>]

On 06/06/2026 17:12, Tu Dinh wrote:
> On 05/06/2026 17:22, Owen Smith wrote:
>> Fix an issue where all RingWatchdog threads were not getting stopped when a
>> REMOVE_DEVICE is processed. This issue does not hit in normal operation where
>> XenVbd will go through the system power transitions instead of receiving
>> IRP_MN_REMOVE_DEVICE. In normal operations, the boot disk will prevent you
>> from disabling the XenVbd device, and XenVbd will not get 
>> IRP_MN_REMOVE_DEVICE.
>>
>> If XenVbd is enumerating only non-emulateable disks, then it can be removed,
>> which did not stop all RingWatchdog threads. Once these threads attempt to
>> access any data from the now unloaded driver, a 0xCE
>> DRIVER_UNLOADED_WITHOUT_CANCELLING_PENDING_OPERATIONS bugcheck will be 
>> observed.
>>
>> Correct an out-by-one issue with the cleanup code, so all RingWatchdog 
>> threads
>> are correctly stopped before unloading in this case.
>>
> 
> I think it can be solved by just swapping for a post-decrement, i.e.
> "while (Index-- != 0)", while keeping the same intention.
> 
> This pattern of "while (--Index != 0)" looks to be very common in the
> codebase. Luckily it's used mainly in failure paths and driver unloads.
> Would you mind posting a full multiseries that fixes all of these spots?
> 

Never mind, the other places use a "while (--Index >= 0)" pattern with a 
signed Index, which works correctly. So for the sake of consistency, we 
can convert to this pattern as well, even if it does look somewhat 
bug-prone.

But in the same ring.c file, there's a "while (--Index > 0)" in the 
RingCreate cleanup that looks incorrect at first glance. Could you fix 
that too while you're at it?

>> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
>> ---
>>    src/xenvbd/ring.c | 7 ++++---
>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
>> index d05dfc3..6520dff 100644
>> --- a/src/xenvbd/ring.c
>> +++ b/src/xenvbd/ring.c
>> @@ -2503,12 +2503,13 @@ RingDestroy(
>>        IN  PXENVBD_RING    Ring
>>        )
>>    {
>> +    ULONG               MaxQueues;
>>        ULONG               Index;
>>    
>> -    Index = FrontendGetMaxQueues(Ring->Frontend);
>> -    ASSERT3U(Index, !=, 0);
>> +    MaxQueues = FrontendGetMaxQueues(Ring->Frontend);
>> +    ASSERT3U(MaxQueues, !=, 0);
>>    
>> -    while (--Index != 0) {
>> +    for (Index = 0; Index < MaxQueues; ++Index) {
>>            PXENVBD_BLKIF_RING  BlkifRing = Ring->Ring[Index];
>>    
>>            Ring->Ring[Index] = NULL;
> 
> 
> 



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech