| Message ID | 20210826172658.2116840-3-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/virtio: Minor housekeeping patches | expand |
On Thu, Aug 26, 2021 at 07:26:57PM +0200, Philippe Mathieu-Daudé wrote: >virtio_free_region_cache() is called within call_rcu(), >always with a non-NULL argument. Ensure new code keep it >that way by replacing the NULL check by an assertion. >Add a comment this function is called within call_rcu(). > >Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >--- > hw/virtio/virtio.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >index a5214bca612..3a1f6c520cb 100644 >--- a/hw/virtio/virtio.c >+++ b/hw/virtio/virtio.c >@@ -133,12 +133,10 @@ struct VirtQueue > QLIST_ENTRY(VirtQueue) node; > }; > >+/* Called within call_rcu(). */ > static void virtio_free_region_cache(VRingMemoryRegionCaches *caches) > { >- if (!caches) { >- return; >- } >- >+ assert(caches != NULL); > address_space_cache_destroy(&caches->desc); > address_space_cache_destroy(&caches->avail); > address_space_cache_destroy(&caches->used); >-- >2.31.1 > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
On Thu, Aug 26, 2021 at 07:26:57PM +0200, Philippe Mathieu-Daudé wrote: > virtio_free_region_cache() is called within call_rcu(), > always with a non-NULL argument. Ensure new code keep it > that way by replacing the NULL check by an assertion. > Add a comment this function is called within call_rcu(). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/virtio/virtio.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index a5214bca612..3a1f6c520cb 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -133,12 +133,10 @@ struct VirtQueue > QLIST_ENTRY(VirtQueue) node; > }; > > +/* Called within call_rcu(). */ > static void virtio_free_region_cache(VRingMemoryRegionCaches *caches) > { > - if (!caches) { > - return; > - } > - > + assert(caches != NULL); > address_space_cache_destroy(&caches->desc); > address_space_cache_destroy(&caches->avail); > address_space_cache_destroy(&caches->used); Looks like an artifact that was left in when the code was originally introduced in commit c611c76417f52b335ecaab01c61743e3b705eb7c ("virtio: add MemoryListener to cache ring translations"). Paolo could confirm this. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index a5214bca612..3a1f6c520cb 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -133,12 +133,10 @@ struct VirtQueue QLIST_ENTRY(VirtQueue) node; }; +/* Called within call_rcu(). */ static void virtio_free_region_cache(VRingMemoryRegionCaches *caches) { - if (!caches) { - return; - } - + assert(caches != NULL); address_space_cache_destroy(&caches->desc); address_space_cache_destroy(&caches->avail); address_space_cache_destroy(&caches->used);
virtio_free_region_cache() is called within call_rcu(), always with a non-NULL argument. Ensure new code keep it that way by replacing the NULL check by an assertion. Add a comment this function is called within call_rcu(). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/virtio/virtio.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)