On 19.10.20 05:48, Wei Yang wrote: > On Mon, Oct 12, 2020 at 02:53:22PM +0200, David Hildenbrand wrote: >> Let's try to unplug completely offline big blocks first. Then, (if >> enabled via unplug_offline) try to offline and remove whole big blocks. >> >> No locking necessary - we can deal with concurrent onlining/offlining >> just fine. >> >> Note1: This is sub-optimal and might be dangerous in some environments: we >> could end up in an infinite loop when offlining (e.g., long-term pinnings), >> similar as with DIMMs. We'll introduce safe memory hotunplug via >> fake-offlining next, and use this basic mode only when explicitly enabled. >> >> Note2: Without ZONE_MOVABLE, memory unplug will be extremely unreliable >> with bigger block sizes. >> >> 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 | 156 +++++++++++++++++++++++++++++++++++- >> 1 file changed, 155 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c >> index 94cf44b15cbf..6bcd0acbff32 100644 >> --- a/drivers/virtio/virtio_mem.c >> +++ b/drivers/virtio/virtio_mem.c >> @@ -388,6 +388,12 @@ static int virtio_mem_bbm_bb_states_prepare_next_bb(struct virtio_mem *vm) >> _bb_id++) \ >> if (virtio_mem_bbm_get_bb_state(_vm, _bb_id) == _state) >> >> +#define virtio_mem_bbm_for_each_bb_rev(_vm, _bb_id, _state) \ >> + for (_bb_id = vm->bbm.next_bb_id - 1; \ >> + _bb_id >= vm->bbm.first_bb_id && _vm->bbm.bb_count[_state]; \ >> + _bb_id--) \ >> + if (virtio_mem_bbm_get_bb_state(_vm, _bb_id) == _state) >> + >> /* >> * Set the state of a memory block, taking care of the state counter. >> */ >> @@ -685,6 +691,18 @@ static int virtio_mem_sbm_remove_mb(struct virtio_mem *vm, unsigned long mb_id) >> return virtio_mem_remove_memory(vm, addr, size); >> } >> >> +/* >> + * See virtio_mem_remove_memory(): Try to remove all Linux memory blocks covered >> + * by the big block. >> + */ >> +static int virtio_mem_bbm_remove_bb(struct virtio_mem *vm, unsigned long bb_id) >> +{ >> + const uint64_t addr = virtio_mem_bb_id_to_phys(vm, bb_id); >> + const uint64_t size = vm->bbm.bb_size; >> + >> + return virtio_mem_remove_memory(vm, addr, size); >> +} >> + >> /* >> * Try offlining and removing memory from Linux. >> * >> @@ -731,6 +749,19 @@ static int virtio_mem_sbm_offline_and_remove_mb(struct virtio_mem *vm, >> return virtio_mem_offline_and_remove_memory(vm, addr, size); >> } >> >> +/* >> + * See virtio_mem_offline_and_remove_memory(): Try to offline and remove a >> + * all Linux memory blocks covered by the big block. >> + */ >> +static int virtio_mem_bbm_offline_and_remove_bb(struct virtio_mem *vm, >> + unsigned long bb_id) >> +{ >> + const uint64_t addr = virtio_mem_bb_id_to_phys(vm, bb_id); >> + const uint64_t size = vm->bbm.bb_size; >> + >> + return virtio_mem_offline_and_remove_memory(vm, addr, size); >> +} >> + >> /* >> * Trigger the workqueue so the device can perform its magic. >> */ >> @@ -1928,6 +1959,129 @@ static int virtio_mem_sbm_unplug_request(struct virtio_mem *vm, uint64_t diff) >> return rc; >> } >> >> +/* >> + * Try to offline and remove a big block from Linux and unplug it. Will fail >> + * with -EBUSY if some memory is busy and cannot get unplugged. >> + * >> + * Will modify the state of the memory block. Might temporarily drop the >> + * hotplug_mutex. >> + */ >> +static int virtio_mem_bbm_offline_remove_and_unplug_bb(struct virtio_mem *vm, >> + unsigned long bb_id) >> +{ >> + int rc; >> + >> + if (WARN_ON_ONCE(virtio_mem_bbm_get_bb_state(vm, bb_id) != >> + VIRTIO_MEM_BBM_BB_ADDED)) >> + return -EINVAL; >> + >> + rc = virtio_mem_bbm_offline_and_remove_bb(vm, bb_id); >> + if (rc) >> + return rc; >> + >> + rc = virtio_mem_bbm_unplug_bb(vm, bb_id); >> + if (rc) >> + virtio_mem_bbm_set_bb_state(vm, bb_id, >> + VIRTIO_MEM_BBM_BB_PLUGGED); >> + else >> + virtio_mem_bbm_set_bb_state(vm, bb_id, >> + VIRTIO_MEM_BBM_BB_UNUSED); >> + return rc; >> +} >> + >> +/* >> + * Try to remove a big block from Linux and unplug it. Will fail with >> + * -EBUSY if some memory is online. >> + * >> + * Will modify the state of the memory block. >> + */ >> +static int virtio_mem_bbm_remove_and_unplug_bb(struct virtio_mem *vm, >> + unsigned long bb_id) >> +{ >> + int rc; >> + >> + if (WARN_ON_ONCE(virtio_mem_bbm_get_bb_state(vm, bb_id) != >> + VIRTIO_MEM_BBM_BB_ADDED)) >> + return -EINVAL; >> + >> + rc = virtio_mem_bbm_remove_bb(vm, bb_id); >> + if (rc) >> + return -EBUSY; >> + >> + rc = virtio_mem_bbm_unplug_bb(vm, bb_id); >> + if (rc) >> + virtio_mem_bbm_set_bb_state(vm, bb_id, >> + VIRTIO_MEM_BBM_BB_PLUGGED); >> + else >> + virtio_mem_bbm_set_bb_state(vm, bb_id, >> + VIRTIO_MEM_BBM_BB_UNUSED); >> + return rc; >> +} >> + >> +/* >> + * Test if a big block is completely offline. >> + */ >> +static bool virtio_mem_bbm_bb_is_offline(struct virtio_mem *vm, >> + unsigned long bb_id) >> +{ >> + const unsigned long start_pfn = PFN_DOWN(virtio_mem_bb_id_to_phys(vm, bb_id)); >> + const unsigned long nr_pages = PFN_DOWN(vm->bbm.bb_size); >> + unsigned long pfn; >> + >> + for (pfn = start_pfn; pfn < start_pfn + nr_pages; >> + pfn += PAGES_PER_SECTION) { > > Can we do the check with memory block granularity? I had that initially, but the code turned out nicer this way (e.g., PAGES_PER_SECTION). -- Thanks, David / dhildenb