Re: [PATCH 1/1] hv_balloon: Enable hot-add for memblock sizes > 128 Mbytes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29.04.24 17:30, Michael Kelley wrote:
From: Michael Kelley <mhklinux@xxxxxxxxxxx> Sent: Friday, April 26, 2024 9:36 AM
@@ -505,8 +505,9 @@ enum hv_dm_state {

   static __u8 recv_buffer[HV_HYP_PAGE_SIZE];
   static __u8 balloon_up_send_buffer[HV_HYP_PAGE_SIZE];
+static unsigned long ha_chunk_pgs;

Why not stick to PAGES_IN_2M and call this

ha_pages_in_chunk? Much easier to get than "pgs".

OK.  I was trying to keep the new identifier short so that
mechanically substituting it for HA_CHUNK didn't blow up
the line length.


Apart from that looks good. Some helper macros to convert size to chunks
etc. might make the code even more readable.

I'll look at this.  Might help the line length problem too.


I didn't see any particular complexity in converting size to chunks. But
this slightly opaque sequence is repeated in three places:

	new_inc = (residual / HA_CHUNK) * HA_CHUNK;
	if (residual % HA_CHUNK)
		new_inc += HA_CHUNK;

If HA_CHUNK (or the new memblock size based variable) is a
power of 2, then these can become:

	new_inc = ALIGN(residual, HA_CHUNK);

which is a lot better.  I'll make that change, and a couple of other
changes where things are open coded that could be existing
kernel macros.

Cool! Make sure to CC me on v2.


Question:  Is memblock size guaranteed to be a power of 2?  It looks
to be so in the x86 code, but I can't tell on s390 and ppc.  For safety,
I'll add a check in the Hyper-V balloon driver init code, as the
communication with Hyper-V expects a power of 2.

Yes, in memory_dev_init() we make sure that the size is a power of 2, and if it is not, we would panic().

--
Cheers,

David / dhildenb





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux