| Message ID | 1340790841-23349-1-git-send-email-hdoyu@nvidia.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
* Hiroshi DOYU (hdoyu@nvidia.com) wrote: > allo_pdir() is called in smmu_iommu_domain_init() with spin_lock > held. memory allocations in it have to be atomic/unsleepable. > > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com> > Reported-by: Chris Wright <chrisw@sous-sol.org> Acked-by: Chris Wright <chrisw@sous-sol.org> -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/27/2012 03:54 AM, Hiroshi DOYU wrote: > allo_pdir() is called in smmu_iommu_domain_init() with spin_lock > held. memory allocations in it have to be atomic/unsleepable. Presumably this will cause allocation failures in situations with heavy memory pressure, rather than triggering swapping. Is it normal for IOMMU drivers to work that way? If so, I'm fine with it. Otherwise, can the allocations be performed before the spinlock is taken? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 27 Jun 2012 18:59:48 +0200 Stephen Warren <swarren@wwwdotorg.org> wrote: > On 06/27/2012 03:54 AM, Hiroshi DOYU wrote: > > allo_pdir() is called in smmu_iommu_domain_init() with spin_lock > > held. memory allocations in it have to be atomic/unsleepable. > > Presumably this will cause allocation failures in situations with heavy > memory pressure, rather than triggering swapping. Is it normal for IOMMU > drivers to work that way? If so, I'm fine with it. Otherwise, can the > allocations be performed before the spinlock is taken? There are some requirement for atomic allocation from DMA mapping API, but it's not normla for IOMMU, especiall not for iommu_domain_alloc(). It has already used GFP_KERNEL and then, calls ->iommu_domain_init() in it. The update patch v2 follows. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 27, 2012 at 12:54:01PM +0300, Hiroshi DOYU wrote: > allo_pdir() is called in smmu_iommu_domain_init() with spin_lock > held. memory allocations in it have to be atomic/unsleepable. > > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com> > Reported-by: Chris Wright <chrisw@sous-sol.org> > --- > drivers/iommu/tegra-smmu.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > index ecd6790..3f3d09d 100644 > --- a/drivers/iommu/tegra-smmu.c > +++ b/drivers/iommu/tegra-smmu.c > @@ -550,13 +550,13 @@ static int alloc_pdir(struct smmu_as *as) > return 0; > > as->pte_count = devm_kzalloc(smmu->dev, > - sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_KERNEL); > + sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_ATOMIC); > if (!as->pte_count) { > dev_err(smmu->dev, > "failed to allocate smmu_device PTE cunters\n"); > return -ENOMEM; > } > - as->pdir_page = alloc_page(GFP_KERNEL | __GFP_DMA); > + as->pdir_page = alloc_page(GFP_ATOMIC | __GFP_DMA); > if (!as->pdir_page) { > dev_err(smmu->dev, > "failed to allocate smmu_device page directory\n"); Okay, I queued this one for v3.5. It is small enough so that it counts as a fix. The other two patches are 3.6 stuff. Joerg
Hi Joerg, Joerg Roedel <joerg.roedel@amd.com> wrote @ Mon, 2 Jul 2012 11:57:47 +0200: > On Wed, Jun 27, 2012 at 12:54:01PM +0300, Hiroshi DOYU wrote: > > allo_pdir() is called in smmu_iommu_domain_init() with spin_lock > > held. memory allocations in it have to be atomic/unsleepable. > > > > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com> > > Reported-by: Chris Wright <chrisw@sous-sol.org> > > --- > > drivers/iommu/tegra-smmu.c | 4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > > index ecd6790..3f3d09d 100644 > > --- a/drivers/iommu/tegra-smmu.c > > +++ b/drivers/iommu/tegra-smmu.c > > @@ -550,13 +550,13 @@ static int alloc_pdir(struct smmu_as *as) > > return 0; > > > > as->pte_count = devm_kzalloc(smmu->dev, > > - sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_KERNEL); > > + sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_ATOMIC); > > if (!as->pte_count) { > > dev_err(smmu->dev, > > "failed to allocate smmu_device PTE cunters\n"); > > return -ENOMEM; > > } > > - as->pdir_page = alloc_page(GFP_KERNEL | __GFP_DMA); > > + as->pdir_page = alloc_page(GFP_ATOMIC | __GFP_DMA); > > if (!as->pdir_page) { > > dev_err(smmu->dev, > > "failed to allocate smmu_device page directory\n"); > > Okay, I queued this one for v3.5. It is small enough so that it counts > as a fix. The other two patches are 3.6 stuff. The other 2 patehs are the replacement of this one. They are exclusive. Both of them are basically for the same fix. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hiroshi Doyu <hdoyu@nvidia.com> wrote @ Mon, 2 Jul 2012 12:13:26 +0200: > Hi Joerg, > > Joerg Roedel <joerg.roedel@amd.com> wrote @ Mon, 2 Jul 2012 11:57:47 +0200: > > > On Wed, Jun 27, 2012 at 12:54:01PM +0300, Hiroshi DOYU wrote: > > > allo_pdir() is called in smmu_iommu_domain_init() with spin_lock > > > held. memory allocations in it have to be atomic/unsleepable. > > > > > > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com> > > > Reported-by: Chris Wright <chrisw@sous-sol.org> > > > --- > > > drivers/iommu/tegra-smmu.c | 4 ++-- > > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > > > index ecd6790..3f3d09d 100644 > > > --- a/drivers/iommu/tegra-smmu.c > > > +++ b/drivers/iommu/tegra-smmu.c > > > @@ -550,13 +550,13 @@ static int alloc_pdir(struct smmu_as *as) > > > return 0; > > > > > > as->pte_count = devm_kzalloc(smmu->dev, > > > - sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_KERNEL); > > > + sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_ATOMIC); > > > if (!as->pte_count) { > > > dev_err(smmu->dev, > > > "failed to allocate smmu_device PTE cunters\n"); > > > return -ENOMEM; > > > } > > > - as->pdir_page = alloc_page(GFP_KERNEL | __GFP_DMA); > > > + as->pdir_page = alloc_page(GFP_ATOMIC | __GFP_DMA); > > > if (!as->pdir_page) { > > > dev_err(smmu->dev, > > > "failed to allocate smmu_device page directory\n"); > > > > Okay, I queued this one for v3.5. It is small enough so that it counts > > as a fix. The other two patches are 3.6 stuff. > > The other 2 patehs are the replacement of this one. They are > exclusive. Both of them are basically for the same fix. I might not get your point. If you plan to take the this(the original) as immediate fix for v3.5 you need tok take the others for v3.6 after *reverting* the original. This would also work. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 02, 2012 at 12:19:03PM +0200, Hiroshi Doyu wrote: > Hiroshi Doyu <hdoyu@nvidia.com> wrote @ Mon, 2 Jul 2012 12:13:26 +0200: > > The other 2 patehs are the replacement of this one. They are > > exclusive. Both of them are basically for the same fix. > > I might not get your point. If you plan to take the this(the original) > as immediate fix for v3.5 you need tok take the others for v3.6 after > *reverting* the original. This would also work. Or you rebase the other two patches on-top of the first patch. Should not be too difficult and you can also include the Ack from Steven while at it :) Joerg
"joerg.roedel@amd.com" <joerg.roedel@amd.com> wrote @ Mon, 2 Jul 2012 12:42:31 +0200: > On Mon, Jul 02, 2012 at 12:19:03PM +0200, Hiroshi Doyu wrote: > > Hiroshi Doyu <hdoyu@nvidia.com> wrote @ Mon, 2 Jul 2012 12:13:26 +0200: > > > The other 2 patehs are the replacement of this one. They are > > > exclusive. Both of them are basically for the same fix. > > > > I might not get your point. If you plan to take the this(the original) > > as immediate fix for v3.5 you need tok take the others for v3.6 after > > *reverting* the original. This would also work. > > Or you rebase the other two patches on-top of the first patch. Should > not be too difficult and you can also include the Ack from Steven while > at it :) I'm sending the following patch again. The first one is for v3.5. The rest goes to v3.6 as Joerg suggested. v3.5 [1/1] iommu/tegra: smmu: Fix unsleepable memory allocation v3.6 [1/3] Revert "iommu/tegra: smmu: Fix unsleepable memory allocation" [2/3] iommu/tegra: smmu: Remove unnecessary sanity check at alloc_pdir() [3/3] iommu/tegra: smmu: Fix unsleepable memory allocation at alloc_pdir() Is this OK? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index ecd6790..3f3d09d 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -550,13 +550,13 @@ static int alloc_pdir(struct smmu_as *as) return 0; as->pte_count = devm_kzalloc(smmu->dev, - sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_KERNEL); + sizeof(as->pte_count[0]) * SMMU_PDIR_COUNT, GFP_ATOMIC); if (!as->pte_count) { dev_err(smmu->dev, "failed to allocate smmu_device PTE cunters\n"); return -ENOMEM; } - as->pdir_page = alloc_page(GFP_KERNEL | __GFP_DMA); + as->pdir_page = alloc_page(GFP_ATOMIC | __GFP_DMA); if (!as->pdir_page) { dev_err(smmu->dev, "failed to allocate smmu_device page directory\n");
allo_pdir() is called in smmu_iommu_domain_init() with spin_lock held. memory allocations in it have to be atomic/unsleepable. Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com> Reported-by: Chris Wright <chrisw@sous-sol.org> --- drivers/iommu/tegra-smmu.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)