From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37578) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhwTs-0004X1-8v for qemu-devel@nongnu.org; Thu, 08 Sep 2016 06:22:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhwTo-0007zY-0f for qemu-devel@nongnu.org; Thu, 08 Sep 2016 06:22:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57535) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhwTn-0007zS-Rk for qemu-devel@nongnu.org; Thu, 08 Sep 2016 06:22:47 -0400 Date: Thu, 8 Sep 2016 18:22:40 +0800 From: Peter Xu Message-ID: <20160908102240.GD28348@pxdev.xzpeter.org> References: <1473226344-28520-1-git-send-email-peterx@redhat.com> <1473226344-28520-3-git-send-email-peterx@redhat.com> <20160907060550.GQ2780@voom.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v3 2/3] memory: generalize iommu_ops.notify_started to notifier_add List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: David Gibson , qemu-devel@nongnu.org, mst@redhat.com, jasowang@redhat.com, vkaplans@redhat.com, alex.williamson@redhat.com, wexu@redhat.com, cornelia.huck@de.ibm.com, dgibson@redhat.com On Wed, Sep 07, 2016 at 12:54:23PM +0200, Paolo Bonzini wrote: > > > On 07/09/2016 08:05, David Gibson wrote: > > On Wed, Sep 07, 2016 at 01:32:23PM +0800, Peter Xu wrote: > >> Considering that we may have multiple IOMMU notifier consumers in the > >> future, converting iommu_ops.notify_{started|stopped} into some more > >> general form. Now we can trap all notifier registerations and > >> deregistrations, rather than only the first ones. > >> > >> Power was leveraging the notifier_{started|stopped}, adding iommu_user > >> field for counting on Power guests to achieve the same goal. > > > > Requiring each vIOMMU frontend to reference count or whatever seems > > like a pain. The semantics of notify_started() were designed to avoid > > that. > > > > Instead I'd suggest a callback which gets tripped any time the logical > > OR of the requested notifications for all current notifiers changes. > > I like the suggestion. Alternatively you could call notify_stopped if > old & ~new is nonzero (and you pass old & ~new), and notify_started if > new & ~old is nonzero (and you pass new & ~old). I think now I understand the point... Then I'd prefer to use David's suggestion. A single notify_changed() looks cleaner. To be more explicit, I would prefer to rename it to notifier_flag_changed(), since notify_changed() looks like to be called every time notifier list changed, but actually it is for monitoring the flags. Thanks, -- peterx