On 2021-07-05 16:21, Jason Gunthorpe wrote:
On Mon, Jul 05, 2021 at 02:47:36PM +0100, Robin Murphy wrote:
On 2021-07-05 11:23, Dan Carpenter wrote:
[ Ancient code, but the bug seems real enough still. -dan ]
Hello Upinder Malhi,
The patch e3cf00d0a87f: "IB/usnic: Add Cisco VIC low-level hardware
driver" from Sep 10, 2013, leads to the following static checker
warning:
drivers/iommu/iommu.c:2482 iommu_map()
warn: sleeping in atomic context
drivers/infiniband/hw/usnic/usnic_uiom.c
244 static int usnic_uiom_map_sorted_intervals(struct list_head *intervals,
245 struct usnic_uiom_reg *uiomr)
This function is always called from usnic_uiom_reg_get() which is holding
spin_lock(&pd->lock); so it can't sleep.
FWIW back in those days it wasn't really well defined whether iommu_map()
was callable from non-sleeping contexts (the arch/arm DMA API code relied on
it, for instance). It was only formalised 2 years ago by 781ca2de89ba
("iommu: Add gfp parameter to iommu_ops::map") which introduced the
might_sleep() check that's firing there. I guess these calls want to be
updated to iommu_map_atomic() now.
Does this mean this driver doesn't work at all upstream? I would be
quite interested to delete it.
I think the only time it's actually in trouble is on AMD hardware if one
of those iommu_map() calls has to allocate a new pagetable page and that
allocation has to go down whichever reclaim path actually sleeps.
Historically all the other IOMMU drivers it might have come into contact
with already used GFP_ATOMIC for their internal allocations anyway (AMD
was the only one using a mutex instead of a spinlock internally), and
some like intel-iommu still haven't relaxed that even now.
Robin.
