| Message ID | 20201012125323.17509-22-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-mem: Big Block Mode (BBM) | expand |
On Mon, Oct 12, 2020 at 02:53:15PM +0200, David Hildenbrand wrote: >Let's rename accordingly. > >Cc: "Michael S. Tsirkin" <mst@redhat.com> >Cc: Jason Wang <jasowang@redhat.com> >Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com> >Signed-off-by: David Hildenbrand <david@redhat.com> >--- > drivers/virtio/virtio_mem.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > >diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c >index 3a772714fec9..d06c8760b337 100644 >--- a/drivers/virtio/virtio_mem.c >+++ b/drivers/virtio/virtio_mem.c >@@ -589,8 +589,8 @@ static bool virtio_mem_contains_range(struct virtio_mem *vm, uint64_t start, > return start >= vm->addr && start + size <= vm->addr + vm->region_size; > } > >-static int virtio_mem_notify_going_online(struct virtio_mem *vm, >- unsigned long mb_id) >+static int virtio_mem_sbm_notify_going_online(struct virtio_mem *vm, >+ unsigned long mb_id) Look into this patch with "virtio-mem: Big Block Mode (BBM) memory hotplug" together, I thought the code is a little "complex". The final logic of virtio_mem_memory_notifier_cb() looks like this: virtio_mem_memory_notifier_cb() if (vm->in_sbm) notify_xxx() if (vm->in_sbm) notify_xxx() Can we adjust this like virtio_mem_memory_notifier_cb() notify_xxx() if (vm->in_sbm) return notify_xxx() if (vm->in_sbm) return This style looks a little better to me.
On 19.10.20 03:57, Wei Yang wrote: > On Mon, Oct 12, 2020 at 02:53:15PM +0200, David Hildenbrand wrote: >> Let's rename accordingly. >> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> drivers/virtio/virtio_mem.c | 29 +++++++++++++++-------------- >> 1 file changed, 15 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c >> index 3a772714fec9..d06c8760b337 100644 >> --- a/drivers/virtio/virtio_mem.c >> +++ b/drivers/virtio/virtio_mem.c >> @@ -589,8 +589,8 @@ static bool virtio_mem_contains_range(struct virtio_mem *vm, uint64_t start, >> return start >= vm->addr && start + size <= vm->addr + vm->region_size; >> } >> >> -static int virtio_mem_notify_going_online(struct virtio_mem *vm, >> - unsigned long mb_id) >> +static int virtio_mem_sbm_notify_going_online(struct virtio_mem *vm, >> + unsigned long mb_id) > > Look into this patch with "virtio-mem: Big Block Mode (BBM) memory hotplug" > together, I thought the code is a little "complex". > > The final logic of virtio_mem_memory_notifier_cb() looks like this: > > virtio_mem_memory_notifier_cb() > if (vm->in_sbm) > notify_xxx() > if (vm->in_sbm) > notify_xxx() > > Can we adjust this like > > virtio_mem_memory_notifier_cb() > notify_xxx() > if (vm->in_sbm) > return > notify_xxx() > if (vm->in_sbm) > return > > This style looks a little better to me. Then we lose all the shared code after any of the mode-specific handling? Like we have in MEM_OFFLINE, MEM_ONLINE, MEM_CANCEL_OFFLINE, ... Don't think this will improve the situation.
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c index 3a772714fec9..d06c8760b337 100644 --- a/drivers/virtio/virtio_mem.c +++ b/drivers/virtio/virtio_mem.c @@ -589,8 +589,8 @@ static bool virtio_mem_contains_range(struct virtio_mem *vm, uint64_t start, return start >= vm->addr && start + size <= vm->addr + vm->region_size; } -static int virtio_mem_notify_going_online(struct virtio_mem *vm, - unsigned long mb_id) +static int virtio_mem_sbm_notify_going_online(struct virtio_mem *vm, + unsigned long mb_id) { switch (virtio_mem_sbm_get_mb_state(vm, mb_id)) { case VIRTIO_MEM_SBM_MB_OFFLINE_PARTIAL: @@ -604,8 +604,8 @@ static int virtio_mem_notify_going_online(struct virtio_mem *vm, return NOTIFY_BAD; } -static void virtio_mem_notify_offline(struct virtio_mem *vm, - unsigned long mb_id) +static void virtio_mem_sbm_notify_offline(struct virtio_mem *vm, + unsigned long mb_id) { switch (virtio_mem_sbm_get_mb_state(vm, mb_id)) { case VIRTIO_MEM_SBM_MB_ONLINE_PARTIAL: @@ -622,7 +622,8 @@ static void virtio_mem_notify_offline(struct virtio_mem *vm, } } -static void virtio_mem_notify_online(struct virtio_mem *vm, unsigned long mb_id) +static void virtio_mem_sbm_notify_online(struct virtio_mem *vm, + unsigned long mb_id) { switch (virtio_mem_sbm_get_mb_state(vm, mb_id)) { case VIRTIO_MEM_SBM_MB_OFFLINE_PARTIAL: @@ -639,8 +640,8 @@ static void virtio_mem_notify_online(struct virtio_mem *vm, unsigned long mb_id) } } -static void virtio_mem_notify_going_offline(struct virtio_mem *vm, - unsigned long mb_id) +static void virtio_mem_sbm_notify_going_offline(struct virtio_mem *vm, + unsigned long mb_id) { const unsigned long nr_pages = PFN_DOWN(vm->sbm.sb_size); unsigned long pfn; @@ -655,8 +656,8 @@ static void virtio_mem_notify_going_offline(struct virtio_mem *vm, } } -static void virtio_mem_notify_cancel_offline(struct virtio_mem *vm, - unsigned long mb_id) +static void virtio_mem_sbm_notify_cancel_offline(struct virtio_mem *vm, + unsigned long mb_id) { const unsigned long nr_pages = PFN_DOWN(vm->sbm.sb_size); unsigned long pfn; @@ -716,7 +717,7 @@ static int virtio_mem_memory_notifier_cb(struct notifier_block *nb, break; } vm->hotplug_active = true; - virtio_mem_notify_going_offline(vm, mb_id); + virtio_mem_sbm_notify_going_offline(vm, mb_id); break; case MEM_GOING_ONLINE: mutex_lock(&vm->hotplug_mutex); @@ -726,10 +727,10 @@ static int virtio_mem_memory_notifier_cb(struct notifier_block *nb, break; } vm->hotplug_active = true; - rc = virtio_mem_notify_going_online(vm, mb_id); + rc = virtio_mem_sbm_notify_going_online(vm, mb_id); break; case MEM_OFFLINE: - virtio_mem_notify_offline(vm, mb_id); + virtio_mem_sbm_notify_offline(vm, mb_id); atomic64_add(size, &vm->offline_size); /* @@ -743,7 +744,7 @@ static int virtio_mem_memory_notifier_cb(struct notifier_block *nb, mutex_unlock(&vm->hotplug_mutex); break; case MEM_ONLINE: - virtio_mem_notify_online(vm, mb_id); + virtio_mem_sbm_notify_online(vm, mb_id); atomic64_sub(size, &vm->offline_size); /* @@ -762,7 +763,7 @@ static int virtio_mem_memory_notifier_cb(struct notifier_block *nb, case MEM_CANCEL_OFFLINE: if (!vm->hotplug_active) break; - virtio_mem_notify_cancel_offline(vm, mb_id); + virtio_mem_sbm_notify_cancel_offline(vm, mb_id); vm->hotplug_active = false; mutex_unlock(&vm->hotplug_mutex); break;
Let's rename accordingly. Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- drivers/virtio/virtio_mem.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)