Re: [XENBUS PATCH 3/4] balloon: Stop using fake MDL to store PFN array

[Owen Smith <owen.smith@xxxxxxxxx>]

Sounds good, just checking the ASSERT isnt vital

Reviewed-by: Owen Smith <owen.smith@xxxxxxxxx>

On Thu, Aug 14, 2025 at 11:21 AM Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> wrote:

On 14/08/2025 11:37, Owen Smith wrote:
>
>
> On Wed, Aug 13, 2025 at 1:46 PM Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> wrote:
>
>     @@ -213,90 +208,31 @@ BalloonAllocatePagesForMdl(
>           SkipBytes.QuadPart = 0ull;
>           TotalBytes = (SIZE_T)Count << PAGE_SHIFT;
>
>     -    Mdl = MmAllocatePagesForMdlEx(LowAddress,
>     -                                  HighAddress,
>     -                                  SkipBytes,
>     -                                  TotalBytes,
>     -                                  MmCached,
>     -                                  MM_DONT_ZERO_ALLOCATION |
>     -                                  MM_ALLOCATE_PREFER_CONTIGUOUS |
>     -                                  MM_ALLOCATE_AND_HOT_REMOVE);
>     -    if (Mdl == NULL)
>     -        goto done;
>     -
>     -    ASSERT((Mdl->MdlFlags & (MDL_MAPPED_TO_SYSTEM_VA |
>     -                             MDL_PARTIAL_HAS_BEEN_MAPPED |
>     -                             MDL_PARTIAL |
>     -                             MDL_PARENT_MAPPED_SYSTEM_VA |
>     -                             MDL_SOURCE_IS_NONPAGED_POOL)) == 0);
>     -
>     -done:
>     -    return Mdl;
>     +    return MmAllocatePagesForMdlEx(LowAddress,
>     +                                   HighAddress,
>     +                                   SkipBytes,
>     +                                   TotalBytes,
>     +                                   MmCached,
>     +                                   MM_DONT_ZERO_ALLOCATION |
>     +                                   MM_ALLOCATE_PREFER_CONTIGUOUS |
>     +                                   MM_ALLOCATE_AND_HOT_REMOVE);
>       }
>
> The only change in this chunk is skipping the ASSERT - how important is
> the ASSERT?
>
> Owen

The MDL is taken straight from
MmAllocatePagesForMdlEx(MM_ALLOCATE_AND_HOT_REMOVE), so the assert
doesn't look important at all to me. I've tested a debug build with
verifier and saw no crashes.


Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech