On 17/12/2024 09:38, Avihai Horon wrote:
>>>>> +bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase
>>>>> *bcontainer)
>>>>> {
>>>>> - VFIODevice *vbasedev;
>>>>> -
>>>>> - if (!migration_is_active()) {
>>>>> + if (!migration_is_running()) {
>>>>> return false;
>>>>> }
>>>>>
>>>>> - QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>>>>> - VFIOMigration *migration = vbasedev->migration;
>>>>> -
>>>>> - if (!migration) {
>>>>> - return false;
>>>>> - }
>>>>> -
>>>>> - if (vfio_device_state_is_running(vbasedev) ||
>>>>> - vfio_device_state_is_precopy(vbasedev)) {
>>>>> - continue;
>>>>> - } else {
>>>>> - return false;
>>>>> - }
>>>> Functionally the change implies that even if non-migratable VFIO devices
>>>> behind
>>>> IOMMUs with dirty tracking would still sync DMA bitmap. I think this is OK
>>>> as it
>>>> increases the coverage for calc-dirty-rate (provided my comment in an
>>>> earlier
>>>> patch) such that if you try to get a dirty rate included the IOMMU
>>>> invalidations
>>>> marking the bits accordingly.
>>> We still have the "if (!migration_is_running())" check above, so
>>> non-migratable
>>> VFIO devices won't sync.
>>> But that's a valid point for when we'll allow VFIO log syncs for clac-dirty-
>>> rate.
>>>
>> It's the other way around :) This change helps calc-dirty-rate because you
>> can
>> use it and still account for DMA unmap based dirties.
>>
>> migration_is_running just stops logs if migration is not running. And that
>> doesn't care about VFIO migation support.
>>
>> But if migration is running, whether the device supports migration or not...
>
> If the device doesn't support migration then migration can't run, no?
/facepalm yes :D
We still have migration blockers
> But anyway, as we talked in the other thread, I can untie this from migration
> and then, as you said above, it may also dirty sync for non-migratable devices
> which will make calc-dirty-rate more accurate.
>
Yeap
