From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean-Philippe Brucker Subject: Re: [PATCH 07/37] iommu: Add a page fault handler Date: Wed, 14 Mar 2018 13:08:07 +0000 Message-ID: References: <20180212183352.22730-1-jean-philippe.brucker@arm.com> <20180212183352.22730-8-jean-philippe.brucker@arm.com> <20180308164035.000065c2@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180308164035.000065c2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Jonathan Cameron Cc: Mark Rutland , "ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , "kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org" , Will Deacon , "okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" , "ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org" , "bharatku-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org" , "linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Catalin Marinas , "rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org" , "lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org" , rjw@rjwyso List-Id: devicetree@vger.kernel.org Hi Jonathan, Thanks for reviewing On 08/03/18 15:40, Jonathan Cameron wrote: >> +/** >> + * iommu_fault_queue_unregister() - Unregister an IOMMU driver from the fault >> + * queue. >> + * @flush_notifier: same parameter as iommu_fault_queue_register >> + */ >> +void iommu_fault_queue_unregister(struct notifier_block *flush_notifier) >> +{ >> + down_write(&iommu_fault_queue_sem); >> + if (refcount_dec_and_test(&iommu_fault_queue_refs)) { >> + destroy_workqueue(iommu_fault_queue); >> + iommu_fault_queue = NULL; >> + } >> + up_write(&iommu_fault_queue_sem); >> + >> + if (flush_notifier) >> + blocking_notifier_chain_unregister(&iommu_fault_queue_flush_notifiers, >> + flush_notifier); > I would expect the ordering in queue_unregister to be the reverse of queue > register (to make it obvious there are no races). > > That would put this last block at the start before potentially destroying > the work queue. If I'm missing something then perhaps a comment to > explain why the ordering is not the obvious one? Sure, I'll fix the order, I don't think there was any good reason for it Thanks, Jean