On Tue, Jan 17, 2023 at 03:38:51AM +0000, Tian, Kevin wrote: > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > Sent: Saturday, January 7, 2023 12:43 AM > > > > @@ -2368,7 +2372,7 @@ static int iommu_domain_identity_map(struct > > dmar_domain *domain, > > > > return __domain_mapping(domain, first_vpfn, > > first_vpfn, last_vpfn - first_vpfn + 1, > > - DMA_PTE_READ|DMA_PTE_WRITE); > > + DMA_PTE_READ|DMA_PTE_WRITE, > > GFP_KERNEL); > > } > > Baolu, can you help confirm whether switching from GFP_ATOMIC to > GFP_KERNEL is OK in this path? it looks fine to me in a quick glance > but want to be conservative here. I checked it carefully myself as well, good to check again. > > @@ -4333,7 +4337,8 @@ static size_t intel_iommu_unmap(struct > > iommu_domain *domain, > > > > /* Cope with horrid API which requires us to unmap more than the > > size argument if it happens to be a large-page mapping. */ > > - BUG_ON(!pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, > > &level)); > > + BUG_ON(!pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, > > &level, > > + GFP_ATOMIC)); > > with level==0 it implies it's only lookup w/o pgtable allocation. From this > angle it reads better to use a more relaxed gfp e.g. GFP_KERNEL here. We should only write GFP_KERNEL if it is actually a sleepable context because it will be mighty confusing if it isn't. I couldn't tell what the context is so I left it as ATOMIC. You are correct this is only just a lookup and so the value is never used / doesn't matter. Jason
