On Jul 27 16:16, Jinhao Fan wrote: > at 3:06 PM, Klaus Jensen <i...@irrelevant.dk> wrote: > > > On Jul 26 14:08, Klaus Jensen wrote: > >> Alright. Forget about the iommu, that was just a coincidence. > >> > >> This patch seems to fix it. I guess it is the > >> event_notifier_set_handler(..., NULL) that does the trick, but I'd like > >> to understand why ;) > >> > >> > >> diff --git i/hw/nvme/ctrl.c w/hw/nvme/ctrl.c > >> index 533ad14e7a61..3bc3c6bfbe78 100644 > >> --- i/hw/nvme/ctrl.c > >> +++ w/hw/nvme/ctrl.c > >> @@ -4238,7 +4238,9 @@ static void nvme_cq_notifier(EventNotifier *e) > >> NvmeCQueue *cq = container_of(e, NvmeCQueue, notifier); > >> NvmeCtrl *n = cq->ctrl; > >> > >> - event_notifier_test_and_clear(&cq->notifier); > >> + if (!event_notifier_test_and_clear(e)) { > >> + return; > >> + } > >> > >> nvme_update_cq_head(cq); > >> > >> @@ -4275,7 +4277,9 @@ static void nvme_sq_notifier(EventNotifier *e) > >> { > >> NvmeSQueue *sq = container_of(e, NvmeSQueue, notifier); > >> > >> - event_notifier_test_and_clear(&sq->notifier); > >> + if (!event_notifier_test_and_clear(e)) { > >> + return; > >> + } > >> > > Yes, virtio also checks the return value of event_notifier_test_and_clear(). > > >> nvme_process_sq(sq); > >> } > >> @@ -4307,6 +4311,8 @@ static void nvme_free_sq(NvmeSQueue *sq, NvmeCtrl *n) > >> if (sq->ioeventfd_enabled) { > >> memory_region_del_eventfd(&n->iomem, > >> 0x1000 + offset, 4, false, 0, > >> &sq->notifier); > >> + event_notifier_set_handler(&sq->notifier, NULL); > >> + nvme_sq_notifier(&sq->notifier); > >> event_notifier_cleanup(&sq->notifier); > >> } > >> g_free(sq->io_req); > >> @@ -4697,6 +4703,8 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n) > >> if (cq->ioeventfd_enabled) { > >> memory_region_del_eventfd(&n->iomem, > >> 0x1000 + offset, 4, false, 0, > >> &cq->notifier); > >> + event_notifier_set_handler(&cq->notifier, NULL); > >> + nvme_cq_notifier(&cq->notifier); > >> event_notifier_cleanup(&cq->notifier); > >> } > >> if (msix_enabled(&n->parent_obj)) { > > > > I’m a bit messed up here. I will further check how virtio handles queue > deletion today.
Yeah, the API documentation is not exactly exhaustive on the specifics here, so I kinda inferred this from the virtio code. If this is the way to do it, then I think I will follow up with a patch for the event notifier api with a not on how teardown should be done. Paolo - get_maintainer.pl spits you out for main-loop.h. Any chance you could shed some light on this?
signature.asc
Description: PGP signature