Re: [PATCH v1 25/29] virtio-mem: Big Block Mode (BBM) memory hotplug

[David Hildenbrand]

On 16.10.20 11:38, Wei Yang wrote:
> On Mon, Oct 12, 2020 at 02:53:19PM +0200, David Hildenbrand wrote:
>> Currently, we do not support device block sizes that exceed the Linux
>> memory block size. For example, having a device block size of 1 GiB (e.g.,
>> gigantic pages in the hypervisor) won't work with 128 MiB Linux memory
>> blocks.
>>
>> Let's implement Big Block Mode (BBM), whereby we add/remove at least
>> one Linux memory block at a time. With a 1 GiB device block size, a Big
>> Block (BB) will cover 8 Linux memory blocks.
>>
>> We'll keep registering the online_page_callback machinery, it will be used
>> for safe memory hotunplug in BBM next.
>>
>> Note: BBM is properly prepared for variable-sized Linux memory
>> blocks that we might see in the future. So we won't care how many Linux
>> memory blocks a big block actually spans, and how the memory notifier is
>> called.
>>
>> Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
>> Cc: Jason Wang <jasowang@xxxxxxxxxx>
>> Cc: Pankaj Gupta <pankaj.gupta.linux@xxxxxxxxx>
>> Cc: Michal Hocko <mhocko@xxxxxxxxxx>
>> Cc: Oscar Salvador <osalvador@xxxxxxx>
>> Cc: Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>> ---
>> drivers/virtio/virtio_mem.c | 484 ++++++++++++++++++++++++++++++------
>> 1 file changed, 402 insertions(+), 82 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index e68d0d99590c..4d396ef98a92 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -30,12 +30,18 @@ MODULE_PARM_DESC(unplug_online, "Try to unplug online memory");
>> /*
>>  * virtio-mem currently supports the following modes of operation:
>>  *
>> - * * Sub Block Mode (SBM): A Linux memory block spans 1..X subblocks (SB). The
>> + * * Sub Block Mode (SBM): A Linux memory block spans 2..X subblocks (SB). The
>>  *   size of a Sub Block (SB) is determined based on the device block size, the
>>  *   pageblock size, and the maximum allocation granularity of the buddy.
>>  *   Subblocks within a Linux memory block might either be plugged or unplugged.
>>  *   Memory is added/removed to Linux MM in Linux memory block granularity.
>>  *
>> + * * Big Block Mode (BBM): A Big Block (BB) spans 1..X Linux memory blocks.
>> + *   Memory is added/removed to Linux MM in Big Block granularity.
>> + *
>> + * The mode is determined automatically based on the Linux memory block size
>> + * and the device block size.
>> + *
>>  * User space / core MM (auto onlining) is responsible for onlining added
>>  * Linux memory blocks - and for selecting a zone. Linux Memory Blocks are
>>  * always onlined separately, and all memory within a Linux memory block is
>> @@ -61,6 +67,19 @@ enum virtio_mem_sbm_mb_state {
>> 	VIRTIO_MEM_SBM_MB_COUNT
>> };
>>
>> +/*
>> + * State of a Big Block (BB) in BBM, covering 1..X Linux memory blocks.
>> + */
>> +enum virtio_mem_bbm_bb_state {
>> +	/* Unplugged, not added to Linux. Can be reused later. */
>> +	VIRTIO_MEM_BBM_BB_UNUSED = 0,
>> +	/* Plugged, not added to Linux. Error on add_memory(). */
>> +	VIRTIO_MEM_BBM_BB_PLUGGED,
>> +	/* Plugged and added to Linux. */
>> +	VIRTIO_MEM_BBM_BB_ADDED,
>> +	VIRTIO_MEM_BBM_BB_COUNT
>> +};
>> +
>> struct virtio_mem {
>> 	struct virtio_device *vdev;
>>
>> @@ -113,6 +132,9 @@ struct virtio_mem {
>> 	atomic64_t offline_size;
>> 	uint64_t offline_threshold;
>>
>> +	/* If set, the driver is in SBM, otherwise in BBM. */
>> +	bool in_sbm;
>> +
>> 	struct {
>> 		/* Id of the first memory block of this device. */
>> 		unsigned long first_mb_id;
>> @@ -151,9 +173,27 @@ struct virtio_mem {
>> 		unsigned long *sb_states;
>> 	} sbm;
>>
>> +	struct {
>> +		/* Id of the first big block of this device. */
>> +		unsigned long first_bb_id;
>> +		/* Id of the last usable big block of this device. */
>> +		unsigned long last_usable_bb_id;
>> +		/* Id of the next device bock to prepare when needed. */
>> +		unsigned long next_bb_id;
>> +
>> +		/* Summary of all big block states. */
>> +		unsigned long bb_count[VIRTIO_MEM_BBM_BB_COUNT];
>> +
>> +		/* One byte state per big block. See sbm.mb_states. */
>> +		uint8_t *bb_states;
>> +
>> +		/* The block size used for (un)plugged, adding/removing. */
>> +		uint64_t bb_size;
>> +	} bbm;
> 
> Can we use a union here?

As I had the same thought initially, it most probably makes sense :)

Thanks!


-- 
Thanks,

David / dhildenb