Re: [PATCH 1/1] virtio_balloon: do not set pr_dev_info.report unconditionally

["David Hildenbrand (Red Hat)" <david@kernel.org>]

On 12/17/25 05:52, Michael Kelley wrote:
> From: Dongli Zhang <dongli.zhang@oracle.com> 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