Re: [PATCH v4 4/4] virtio_balloon: Specify page reporting order if needed

[Gavin Shan <gshan@redhat.com>]

On 6/25/21 3:57 PM, Michael S. Tsirkin wrote:
> On Fri, Jun 25, 2021 at 09:47:10AM +0800, Gavin Shan wrote:
>> The page reporting won't be triggered if the freeing page can't come
>> up with a free area, whose size is equal or bigger than the threshold
>> (page reporting order). The default page reporting order, equal to
>> @pageblock_order, is too huge on some architectures to trigger page
>> reporting. One example is ARM64 when 64KB base page size is used.
>>
>>        PAGE_SIZE:          64KB
>>        pageblock_order:    13       (512MB)
>>        MAX_ORDER:          14
>>
>> This specifies the page reporting order to 5 (2MB) for this specific
>> case so that page reporting can be triggered.
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: virtualization@lists.linux-foundation.org
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
>> ---
>>   drivers/virtio/virtio_balloon.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
>> index 510e9318854d..47dce91f788c 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -993,6 +993,23 @@ static int virtballoon_probe(struct virtio_device *vdev)
>>   			goto out_unregister_oom;
>>   		}
>>   
>> +		/*
>> +		 * The default page reporting order is @pageblock_order, which
>> +		 * corresponds to 512MB in size on ARM64 when 64KB base page
>> +		 * size is used. The page reporting won't be triggered if the
>> +		 * freeing page can't come up with a free area like that huge.
>> +		 * So we specify the page reporting order to 5, corresponding
>> +		 * to 2MB. It helps to avoid THP splitting if 4KB base page
>> +		 * size is used by host.
>> +		 *
>> +		 * Ideally, the page reporting order is selected based on the
>> +		 * host's base page size. However, it needs more work to report
>> +		 * that value. The hard-coded order would be fine currently.
>> +		 */
>> +#if defined(CONFIG_ARM64) && defined(CONFIG_ARM64_64K_PAGES)
>> +		vb->pr_dev_info.order = 5;
>> +#endif
>> +
> 
> I was hoping we can get rid of the hacks in virtio with the new
> parameter and logic in mm core. Why not?
> 

Yes, it's something for future as the comments says. Ideally, The
page reporting order can be provided by VMM (QEMU). However, guest's
memory could be backed up by combinations like 4KB pages, 16KB huge
pages, ..., 1GB huge pages. So I need to sort it out before the hack
can be removed from virtio-balloon.

>>   		err = page_reporting_register(&vb->pr_dev_info);
>>   		if (err)
>>   			goto out_unregister_oom;

Thanks,
Gavin