| Message ID | 20200706211407.20018.51061.stgit@localhost.localdomain |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-balloon: Free page hinting clean-ups | expand |
On Mon, 6 Jul 2020 at 22:15, Alexander Duyck <alexander.duyck@gmail.com> wrote: > > From: Alexander Duyck <alexander.h.duyck@linux.intel.com> > > There is already locking in place when we are stopping free page hinting > but there is not similar protections in place when we start. I can only > assume this was overlooked as in most cases the page hinting should not be > occurring when we are starting the hinting, however there is still a chance > we could be processing hints by the time we get back around to restarting > the hinting so we are better off making sure to protect the state with the > mutex lock rather than just updating the value with no protections. > > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> > --- > hw/virtio/virtio-balloon.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index 0c0fd7114799..b3e96a822b4d 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -593,6 +593,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) > return; > } > > + qemu_mutex_lock(&s->free_page_lock); > + > if (s->free_page_report_cmd_id == UINT_MAX) { > s->free_page_report_cmd_id = > VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN; > @@ -601,6 +603,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) > } > > s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED; > + qemu_mutex_unlock(&s->free_page_lock); > + > virtio_notify_config(vdev); > } Coverity spotted this lack of locking too: CID 1430269. thanks -- PMM
On Thu, 9 Jul 2020 at 16:23, Peter Maydell <peter.maydell@linaro.org> wrote:
Side note: the virtio-dev mailing list produces an error
including a random mysqldb related failure if you try to post
to it as a non-list-member, which makes it awkward to cross-post
to it and to a public list like qemu-devel:
<virtio-dev@lists.oasis-open.org>: permission denied. Command output: Traceback
(most recent call last): File "/home/oasis/bin/kmlm_crosspost_issubn",
line 14, in <module> import MySQLdb ImportError: No module named
MySQLdb Sorry, only subscribers may post. If you are a subscriber, please
forward this message to virtio-dev-owner@lists.oasis-open.org to get your
new address included (#5.7.2) ERROR: postqmail-local Error #77
thanks
-- PMM
On 06.07.20 23:14, Alexander Duyck wrote: > From: Alexander Duyck <alexander.h.duyck@linux.intel.com> > > There is already locking in place when we are stopping free page hinting > but there is not similar protections in place when we start. I can only > assume this was overlooked as in most cases the page hinting should not be > occurring when we are starting the hinting, however there is still a chance > we could be processing hints by the time we get back around to restarting > the hinting so we are better off making sure to protect the state with the > mutex lock rather than just updating the value with no protections. > > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> > --- > hw/virtio/virtio-balloon.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index 0c0fd7114799..b3e96a822b4d 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -593,6 +593,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) > return; > } > > + qemu_mutex_lock(&s->free_page_lock); > + > if (s->free_page_report_cmd_id == UINT_MAX) { > s->free_page_report_cmd_id = > VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN; > @@ -601,6 +603,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) > } > > s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED; > + qemu_mutex_unlock(&s->free_page_lock); > + > virtio_notify_config(vdev); > } > > Yes, makes sense, thanks Acked-by: David Hildenbrand <david@redhat.com>
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 0c0fd7114799..b3e96a822b4d 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -593,6 +593,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) return; } + qemu_mutex_lock(&s->free_page_lock); + if (s->free_page_report_cmd_id == UINT_MAX) { s->free_page_report_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN; @@ -601,6 +603,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) } s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED; + qemu_mutex_unlock(&s->free_page_lock); + virtio_notify_config(vdev); }