From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751329AbbBCMZ5 (ORCPT ); Tue, 3 Feb 2015 07:25:57 -0500 Received: from casper.infradead.org ([85.118.1.10]:36704 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932250AbbBCMZ4 (ORCPT ); Tue, 3 Feb 2015 07:25:56 -0500 Date: Tue, 3 Feb 2015 13:25:51 +0100 From: Peter Zijlstra To: Linus Torvalds Cc: Benjamin LaHaise , linux-aio@kvack.org, Linux Kernel , Joerg Roedel , Jesse Barnes Subject: [PATCH] iommu/amd: Fix amd_iommu_free_device() Message-ID: <20150203122551.GJ24151@twins.programming.kicks-ass.net> References: <20150201144058.GM2974@kvack.org> <20150201221458.GN2974@kvack.org> <20150202001628.GO2974@kvack.org> <20150203112733.GM26304@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150203112733.GM26304@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 03, 2015 at 12:27:33PM +0100, Peter Zijlstra wrote: > drivers/iommu/amd_iommu_v2.c-static void put_device_state_wait(struct device_state *dev_state) > drivers/iommu/amd_iommu_v2.c-{ > drivers/iommu/amd_iommu_v2.c- DEFINE_WAIT(wait); > drivers/iommu/amd_iommu_v2.c- > drivers/iommu/amd_iommu_v2.c- prepare_to_wait(&dev_state->wq, &wait, TASK_UNINTERRUPTIBLE); > drivers/iommu/amd_iommu_v2.c- if (!atomic_dec_and_test(&dev_state->count)) > drivers/iommu/amd_iommu_v2.c: schedule(); > drivers/iommu/amd_iommu_v2.c- finish_wait(&dev_state->wq, &wait); > drivers/iommu/amd_iommu_v2.c- > drivers/iommu/amd_iommu_v2.c- free_device_state(dev_state); > drivers/iommu/amd_iommu_v2.c-} > > No loop... Jesse, any objections to this? --- Subject: iommu/amd: Fix amd_iommu_free_device() put_device_state_wait() doesn't loop on the condition and a spurious wakeup will have it free the device state even though there might still be references out to it. Fix this by using 'normal' wait primitives. Signed-off-by: Peter Zijlstra (Intel) --- drivers/iommu/amd_iommu_v2.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c index 90f70d0e1141..b6398d7285f7 100644 --- a/drivers/iommu/amd_iommu_v2.c +++ b/drivers/iommu/amd_iommu_v2.c @@ -151,18 +151,6 @@ static void put_device_state(struct device_state *dev_state) wake_up(&dev_state->wq); } -static void put_device_state_wait(struct device_state *dev_state) -{ - DEFINE_WAIT(wait); - - prepare_to_wait(&dev_state->wq, &wait, TASK_UNINTERRUPTIBLE); - if (!atomic_dec_and_test(&dev_state->count)) - schedule(); - finish_wait(&dev_state->wq, &wait); - - free_device_state(dev_state); -} - /* Must be called under dev_state->lock */ static struct pasid_state **__get_pasid_state_ptr(struct device_state *dev_state, int pasid, bool alloc) @@ -851,7 +839,13 @@ void amd_iommu_free_device(struct pci_dev *pdev) /* Get rid of any remaining pasid states */ free_pasid_states(dev_state); - put_device_state_wait(dev_state); + put_device_state(dev_state); + /* + * Wait until the last reference is dropped before freeing + * the device state. + */ + wait_event(dev_state->wq, !atomic_read(&dev_state->count)); + free_device_state(dev_state); } EXPORT_SYMBOL(amd_iommu_free_device);