On Aug 15, 2022, at 5:52 AM, Alexander Atanasov <alexander.atanasov@xxxxxxxxxxxxx> wrote: > ⚠ External Email > > Hi, > > On 9.08.22 20:44, Nadav Amit wrote: >> On Aug 9, 2022, at 2:53 AM, Alexander Atanasov <alexander.atanasov@xxxxxxxxxxxxx> wrote: >> >>> Update the value in page_alloc on balloon fill/leak. >> >> Some general comments if this patch goes forward. >> >> Please cc pv-drivers@xxxxxxxxxx in the future. > > Ok. > >>> Cc: David Hildenbrand <david@xxxxxxxxxx> >>> Cc: Wei Liu <wei.liu@xxxxxxxxxx> >>> Cc: Nadav Amit <namit@xxxxxxxxxx> >>> >>> Signed-off-by: Alexander Atanasov <alexander.atanasov@xxxxxxxxxxxxx> >>> --- >>> drivers/virtio/virtio_balloon.c | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> Firts user, other balloons i will do if it is accepted to avoid too much emails. >>> >>> >>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c >>> index b9737da6c4dd..e2693ffbd48b 100644 >>> --- a/drivers/virtio/virtio_balloon.c >>> +++ b/drivers/virtio/virtio_balloon.c >>> @@ -208,6 +208,16 @@ static void set_page_pfns(struct virtio_balloon *vb, >>> page_to_balloon_pfn(page) + i); >>> } >>> >>> +static void update_meminfo(struct virtio_balloon *vb) >> >> Putting aside the less-than-optimal function name, I would like to ask that > > Right, i will think of a better one. > >> any new generic balloon logic would go into balloon_compaction.[hc] as much > > If it is going to be a place for generic logic may be it should be > renamed to balloon_common.[ch] ? > >> as possible. I made the effort to reuse this infrastructure (which is now >> used by both VMware and virtio), and would prefer to share as much code as >> possible. >> >> For instance, I would appreciate if the update upon inflate would go into >> balloon_page_list_enqueue() and balloon_page_enqueue(). VMware's 2MB pages >> logic is not shared, so it would require a change that is specific for >> VMware code. > > I looked at the code and i do not see how i can reuse it since > BALLOON_COMPACTION can be disabled and as you say even for VMWare it > would require updates on other places. Looks like if i do so i would > have to handle update from each driver for both cases. I think it is > better to clearly mark the spots when drivers do their internal > recalculations and report to the core. I see only VMWare is using > balloon_page_list_enqueue , virtio balloon is using only migration and > i don't see how to hook it there - i haven't checked the rest of the > balloons but i guess it would be similiar . I agree it is a good to have > a common place for such logic but it might be better of for a separate > work in the future. Fine. I would live with whatever you do and if needed change it later. Thanks for considering. Regards, Nadav _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
