Re: [PATCH v2] vfio: do not coalesce DMA mappings

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 5/10/2023 1:58 PM, Nipun Gupta wrote:
> 
> 
> On 4/24/2023 8:52 PM, David Marchand wrote:
>>
>> Hello Anatoly,
>>
>> On Wed, Apr 5, 2023 at 4:17 PM Burakov, Anatoly
>> <anatoly.burakov@intel.com> wrote:
>>>>> Could you please provide some steps to reproduce the hotplug issue
>>>>> you're having? It would be great to have a test case for this patchset
>>>>> to put it in context.
>>>>
>>>> I am working on CDX bus
>>>> (http://patchwork.dpdk.org/project/dpdk/patch/20230124140746.594066-2-nipun.gupta@amd.com/) and trying out some cases for plug/unplug.
>>>>
>>>> The test is as follows:
>>>>     # Run testpmd application
>>>>     ./dpdk-testpmd -c 0x3 -- -i --nb-cores=1
>>>>
>>>>     # Bind to VFIO
>>>>     echo "vfio-cdx" >  /sys/bus/cdx/devices/cdx-00\:00/driver_override
>>>>     echo "cdx-00:00" > /sys/bus/cdx/drivers_probe
>>>>
>>>>     # Plug a device
>>>>     testpmd> port attach cdx:cdx-00:00
>>>>
>>>>     #quit testpmd
>>>>     testpmd> quit
>>>>
>>>> This gave error at testpmd exit that memory cannot be freed. On
>>>> debugging I updated this code and seems it should be seen with any of
>>>> the device.
>>>>
>>>> I see similar test case (without quit) mentioned
>>>> https://doc.dpdk.org/dts/test_plans/hotplug_test_plan.html, but the
>>>> difference is that it is with igb_uio and issue is being observed with
>>>> VFIO.
>>>>
>>>> Please note the device/bus mentioned in the commands is not yet
>>>> upstreamed in DPDK, but patches would be sent out soon.
>>>>
>>>> Thanks,
>>>> Nipun
>>>>
>>>
>>> Thanks, I can reproduce this issue with regular devices too (run testpmd
>>> with no devices, bind a NIC to VFIO, attach it, then quit). You're
>>> correct in that since the initial mapping was done with mapping large
>>> contiguous zones (such as when mempools are created before attach), any
>>> subsequent freeing of memory will cause these errors to happen.
>>>
>>> I don't think this can be fixed by anything other than not doing the
>>> contiguous mapping thing, so provisionally, I think this patch should be
>>> accepted. I'll play around with it some more and get back to you :)
>>
>> Can we conclude on this topic?
>> It is best we merge this kind of change the sooner possible for a 
>> release.
> 
> Hi Anatoly,
>      Can you kindly update on this?
> 

Hi all,

apologies for late reply.

Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly