Re: [PATCH] Standardize blkif ring cleanup iteration

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

On 08/06/2026 16:50, Owen Smith wrote:
> Use post-decrement when walking back through the ring array calling
> blkif ring cleanup calls, so that the 0-th element is not missed.

Minor comment: This line about post-decrement should be updated.

Otherwise:

Reviewed-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>

> 
> This is especially important for the BlkifRingDestroy call, which
> needs to stop the RingWatchdog thread during IRP_MN_REMOVE_DEVICE.
> Normally, the boot disk prevents the PnP manager issuing the
> IRP_MN_REMOVE_DEVICE IRP so the following problem doesnt occur,
> but when the boot disk is emulated, its possible to get the IRP and
> have xenvbd unload. In this case, the RingWatchdog thread can still
> be scheduled for blkif ring-0, which results in a page fault, and
> 0xCE DRIVER_UNLOADED_WITHOUT_CANCELLING_PENDING_OPERATIONS bugcheck.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>   src/xenvbd/ring.c | 41 ++++++++++++++++-------------------------
>   1 file changed, 16 insertions(+), 25 deletions(-)
> 
> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
> index d05dfc3..d7daaab 100644
> --- a/src/xenvbd/ring.c
> +++ b/src/xenvbd/ring.c
> @@ -2418,7 +2418,7 @@ RingCreate(
>       PXENVBD_TARGET          Target = FrontendGetTarget(Frontend);
>       PXENVBD_ADAPTER         Adapter = TargetGetAdapter(Target);
>       ULONG                   MaxQueues;
> -    ULONG                   Index;
> +    LONG                    Index;
>       NTSTATUS                status;
>   
>       *Ring = __RingAllocate(sizeof(XENVBD_RING));
> @@ -2447,7 +2447,7 @@ RingCreate(
>           goto fail2;
>   
>       Index = 0;
> -    while (Index < MaxQueues) {
> +    while (Index < (LONG) MaxQueues) {
>           PXENVBD_BLKIF_RING  BlkifRing;
>   
>           status = BlkifRingCreate(*Ring, Index, &BlkifRing);
> @@ -2463,7 +2463,7 @@ RingCreate(
>   fail3:
>       Error("fail3\n");
>   
> -    while (--Index > 0) {
> +    while (--Index >= 0) {
>           PXENVBD_BLKIF_RING  BlkifRing = (*Ring)->Ring[Index];
>   
>           (*Ring)->Ring[Index] = NULL;
> @@ -2503,12 +2503,12 @@ RingDestroy(
>       IN  PXENVBD_RING    Ring
>       )
>   {
> -    ULONG               Index;
> +    LONG                Index;
>   
> -    Index = FrontendGetMaxQueues(Ring->Frontend);
> +    Index = (LONG) FrontendGetMaxQueues(Ring->Frontend);
>       ASSERT3U(Index, !=, 0);
>   
> -    while (--Index != 0) {
> +    while (--Index >= 0) {
>           PXENVBD_BLKIF_RING  BlkifRing = Ring->Ring[Index];
>   
>           Ring->Ring[Index] = NULL;
> @@ -2543,8 +2543,8 @@ RingConnect(
>       IN  PXENVBD_RING    Ring
>       )
>   {
> -    ULONG               MaxQueues;
> -    ULONG               Index;
> +    LONG                MaxQueues;
> +    LONG                Index;
>       PCHAR               Buffer;
>       NTSTATUS            status;
>   
> @@ -2619,11 +2619,8 @@ fail6:
>   fail5:
>       Error("fail5\n");
>   
> -    while (Index != 0) {
> -        PXENVBD_BLKIF_RING  BlkifRing;
> -
> -        --Index;
> -        BlkifRing = Ring->Ring[Index];
> +    while (--Index >= 0) {
> +        PXENVBD_BLKIF_RING  BlkifRing = Ring->Ring[Index];
>   
>           BlkifRingDisconnect(BlkifRing);
>       }
> @@ -2736,14 +2733,12 @@ RingDisable(
>       IN  PXENVBD_RING    Ring
>       )
>   {
> -    ULONG               Index;
> +    LONG                Index;
>   
>       Index = FrontendGetNumQueues(Ring->Frontend);
> -    while (Index != 0) {
> -        PXENVBD_BLKIF_RING  BlkifRing;
> +    while (--Index >= 0) {
> +        PXENVBD_BLKIF_RING  BlkifRing = Ring->Ring[Index];
>   
> -        --Index;
> -        BlkifRing = Ring->Ring[Index];
>           BlkifRingDisable(BlkifRing);
>       }
>   }
> @@ -2753,7 +2748,7 @@ RingDisconnect(
>       IN  PXENVBD_RING    Ring
>       )
>   {
> -    ULONG               Index;
> +    LONG                Index;
>   
>       XENBUS_DEBUG(Deregister,
>                    &Ring->DebugInterface,
> @@ -2761,12 +2756,8 @@ RingDisconnect(
>       Ring->DebugCallback = NULL;
>   
>       Index = FrontendGetNumQueues(Ring->Frontend);
> -
> -    while (Index != 0) {
> -        PXENVBD_BLKIF_RING  BlkifRing;
> -
> -        --Index;
> -        BlkifRing = Ring->Ring[Index];
> +    while (--Index >= 0) {
> +        PXENVBD_BLKIF_RING  BlkifRing = Ring->Ring[Index];
>   
>           BlkifRingDisconnect(BlkifRing);
>       }



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech