On 12/17/25 05:52, Michael Kelley wrote:
From: Dongli Zhang <[email protected]> Sent: Wednesday, December 10, 2025
11:10 AM
Hi David,
On 12/10/25 12:09 AM, David Hildenbrand (Red Hat) wrote:
On 12/9/25 22:23, Dongli Zhang wrote:
Do not set vb->pr_dev_info.report unconditionally if
VIRTIO_BALLOON_F_REPORTING is not available.
Can you share with us why you think that should be done? Please document the
"why" and not only the "what".
Without VIRTIO_BALLOON_F_REPORTING, we'll never call page_reporting_register(),
so it will never be used.
But the compiler cannot optimize it out. It only happens during driver loading,
so I am not sure it is worth the churn?
When I was reading about the free-page reporting feature in virtio-balloon, I
was confused as to why pr_dev_info.report was always configured unconditionally.
Later, I looked at the implementation in the Hyper-V balloon driver and noticed
that it even resets pr_dev_info.report back to NULL if page_reporting_register()
fails (see line 1669).
The Hyper-V balloon driver does this because it uses the NULL in
pr_dev_info.report
to indicate if page_reporting_unregister() should be called when the driver
exits.
See disable_page_reporting(). Unlike the virtio balloon driver, the Hyper-V
balloon_probe() function succeeds even if page_reporting_register() fails, so
some indicator is needed on exit. I didn't look super carefully, but it
appears the
virtio balloon driver doesn't need such an indicator.
Nothing is broken AFAIKT.
The patch description should be updated to reflect why we would want to
do that given that nothing is broken.
No strong opinion on the patch with an updated patch description from my
side.
--
Cheers
David
