quiet down swiotlb warnings
PATCH 1/1 quiet down swiotlb warnings
It turns out that the qla2xxx driver sometimes fills up the iotlb on purpose and throttles itself when pci_map_sg() fails. In the case of a driver that expects and handles pci_map_sg() failures, we should not spam the user's console with swiotlb full messages.
If the user runs a driver that cannot handle the swiotlb filling up, 5 warnings should be enough to leave a clue.
lib/swiotlb.c renamed from lib/swiotlb.c.quiet +4 -2
@@ -505,6 +505,7 @@ swiotlb_free_coherent(struct device *hwdstatic void swiotlb_full (struct device *dev,size_t size,int dir,int do_panic) { +static int warnings =0 ;/* * Ran out of IOMMU space for this operation. This is very bad. * Unfortunately the drivers cannot handle this operation properly.
@@ -512,8 +513,9 @@ swiotlb_full(struct device *dev, size_t* When the mapping is small enough return a static buffer to limit * the damage, or panic when the transfer is too big. */ -printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " -"device %s\n" , size, dev ? dev->bus_id :"?" ); +if (++warnings <5 ) +printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " +"device %s\n" , size, dev ? dev->bus_id :"?" );if (size > io_tlb_overflow && do_panic) {if (dir ==DMA_FROM_DEVICE || dir ==DMA_BIDIRECTIONAL )
Quoted 4 lines · Rik van Riel
It turns out that the qla2xxx driver sometimes fills up the iotlb on purpose and throttles itself when
pci_map_sg()fails. In the case of a driver that expects and handlespci_map_sg()failures, we should not spam the user's console with swiotlb full messages.
Why does it do that? Could we supply a better interface for whatever it is trying to do here?
lib/swiotlb.c swiotlb_full(struct device *dev, size_t · line 518
Quoted 6 lines
lib/swiotlb.c renamed from lib/swiotlb.c.quiet +3
-2*/ -printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " -"device %s\n" , size, dev ? dev->bus_id :"?" ); +if (++warnings <5 ) +printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " +"device %s\n" , size, dev ? dev->bus_id :"?" );
Bad idea imho. swiotlb mappings should always lead to printk by default because it is pretty dangerous.
One possible solution for this I could think of would be to define a new pci_map_sg_couldfail() or similar that doesn't warn and use a weak fallback just calling pci_map_sg on other IOMMU implementations.
-Andi
Quoted 6 lines · Andi Kleen
Bad idea imho. swiotlb mappings should always lead to printk by default because it is pretty dangerous.
One possible solution for this I could think of would be to define a new
pci_map_sg_couldfail()or similar that doesn't warn and use a weak fallback just callingpci_map_sgon other IOMMU implementations.
pci_map_sg is defined to be failing when running out of ressources, which is perfectly fine. We don't printk on kmalloc failures either (actually in some cases which is highly annoying and leads people to stick a __GFP_NOWARN into various places)
Quoted 9 lines · Andi Kleen
Quoted 4 lines · Rik van Riel
It turns out that the qla2xxx driver sometimes fills up the iotlb on purpose and throttles itself when
pci_map_sg()fails. In the case of a driver that expects and handlespci_map_sg()failures, we should not spam the user's console with swiotlb full messages.Why does it do that? Could we supply a better interface for whatever it is trying to do here?
The driver only calls pci_map_sg() once it's insured that all local driver resources are available to submit an I/O to the hardware.
Quoted 8 lines
lib/swiotlb.c swiotlb_full(struct device *dev, size_t · line 518
Quoted 5 lines
lib/swiotlb.c renamed from lib/swiotlb.c.quiet +3
-2-printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " -"device %s\n" , size, dev ? dev->bus_id :"?" ); +if (++warnings <5 ) +printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " +"device %s\n" , size, dev ? dev->bus_id :"?" );Bad idea imho. swiotlb mappings should always lead to printk by default because it is pretty dangerous.
Why? It's just another resource which is consumed -- the qla2xxx driver is the final consumer before I/O is submitted out on the wire. The mappings are held for the shorted time required -- as such, are released as soon as the I/O completes.
Quoted 3 lines
One possible solution for this I could think of would be to define a new
pci_map_sg_couldfail()or similar that doesn't warn and use a weak fallback just callingpci_map_sgon other IOMMU implementations.
Quoted 4 lines · Christoph Hellwig
Quoted 3 lines · Andi Kleen
Bad idea imho. swiotlb mappings should always lead to printk by default because it is pretty dangerous.
Quoted 8 lines
Quoted 3 lines
One possible solution for this I could think of would be to define a new
pci_map_sg_couldfail()or similar that doesn't warn and use a weak fallback just callingpci_map_sgon other IOMMU implementations.
pci_map_sgis defined to be failing when running out of ressources, which is perfectly fine. We don't printk on kmalloc failures either (actually in some cases which is highly annoying and leads people to stick a__GFP_NOWARNinto various places)
Andi, I could see your "pretty dangerous" case applying when do_panic is set, but not in any other circumstances.
Does the patch below look better to you?
lib/swiotlb.c renamed from lib/swiotlb.c.quiet +4 -2
@@ -505,6 +505,7 @@ swiotlb_free_coherent(struct device *hwdstatic void swiotlb_full (struct device *dev,size_t size,int dir,int do_panic) { +static int warnings =0 ;/* * Ran out of IOMMU space for this operation. This is very bad. * Unfortunately the drivers cannot handle this operation properly.
@@ -512,8 +513,9 @@ swiotlb_full(struct device *dev, size_t* When the mapping is small enough return a static buffer to limit * the damage, or panic when the transfer is too big. */ -printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " -"device %s\n" , size, dev ? dev->bus_id :"?" ); +if (do_panic || ++warnings <5 ) +printk (KERN_ERR "DMA: Out of SW-IOMMU space for %zu bytes at " +"device %s\n" , size, dev ? dev->bus_id :"?" );if (size > io_tlb_overflow && do_panic) {if (dir ==DMA_FROM_DEVICE || dir ==DMA_BIDIRECTIONAL )
Quoted 10 lines · Christoph Hellwig
Quoted 6 lines · Andi Kleen
Bad idea imho. swiotlb mappings should always lead to printk by default because it is pretty dangerous.
One possible solution for this I could think of would be to define a new
pci_map_sg_couldfail()or similar that doesn't warn and use a weak fallback just callingpci_map_sgon other IOMMU implementations.
pci_map_sgis defined to be failing when running out of ressources, which is perfectly fine. We don't printk on kmalloc failures either (actually
Actually we do as you point out.
Quoted 2 lines
in some cases which is highly annoying and leads people to stick a
__GFP_NOWARNinto various places)
An pci_map_sg failing typically leads to an IO error and we've always printk'ed those. Otherwise people will wonder why they get EIO.
-Andi
Quoted 3 lines · Andi Kleen
An
pci_map_sgfailing typically leads to an IO error and we've always printk'ed those. Otherwise people will wonder why they get EIO.
In some situations. In this case the qla2xxx driver uses the pci_map_sg() failure as a throttling mechanism and printing out all the warnings will actually slow down the system.
Andi, what do you propose as a solution?
Signature · 3 lines
Politics is the struggle between those who want to make their country the best in the world, and those who believe it already is. Each group calls the other unpatriotic.
Quoted 8 lines · Rik van Riel
Quoted 3 lines · Andi Kleen
An
pci_map_sgfailing typically leads to an IO error and we've always printk'ed those. Otherwise people will wonder why they get EIO.In some situations. In this case the qla2xxx driver uses the
pci_map_sg()failure as a throttling mechanism and
First WTF does it need swiotlb anyways? QA hardware should be definitely DAC capable, shouldn't it?
Quoted 2 lines
printing out all the warnings will actually slow down the system.
Another reason is that there is a lot of code that still doesn't check the return values and when that happens you might get data corruption too.
Quoted 2 lines
Andi, what do you propose as a solution?
A different interface; like I wrote in my earlier mail.
Another probabibility would be to have a blocking interface to swiotlb that won't fail. That would be the better solution long term, but i was told it is hard to fit into some current driver interfaces.
-Andi
Quoted 6 lines · Andi Kleen
Quoted 2 lines
in some cases which is highly annoying and leads people to stick a
__GFP_NOWARNinto various places)An
pci_map_sgfailing typically leads to an IO error and we've always printk'ed those. Otherwise people will wonder why they get EIO.
Not in a scsi driver. It will get requeued and the midlayer will submit it again when an outstanding command has finished.
Quoted 7 lines · Andi Kleen
Quoted 6 lines · Rik van Riel
Quoted 3 lines · Andi Kleen
An
pci_map_sgfailing typically leads to an IO error and we've always printk'ed those. Otherwise people will wonder why they get EIO.
Quoted 5 lines
Quoted 2 lines
In some situations. In this case the qla2xxx driver uses the
pci_map_sg()failure as a throttling mechanism andFirst WTF does it need swiotlb anyways? QA hardware should be definitely DAC capable, shouldn't it?
yes, the card can support 64bit DMA transfers. but in this case the 'required' DMA mask returned from dma_get_required_mask() states that a 32bit mask would suffice.
Here's a snippet from the bugzilla report
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219216):
QLogic Fibre Channel HBA Driver
PCI: Enabling device 0000:1f:00.0 (0140 -> 0143)
ACPI: PCI Interrupt 0000:1f:00.0[A] -> GSI 16 (level, low) -> IRQ 16 qla2xxx 0000:1f:00.0: Found an ISP2432, irq 16, iobase 0xffffc20000020000 *** qla2x00_config_dma_addressing: required_mask set to 000000007fffffff. *** qla2x00_config_dma_addressing: required_mask has no high-dword bits set. *** qla2x00_config_dma_addressing: set consistent 64bit mask returned 0. *** qla2x00_config_dma_addressing: defaulting to 32bit mask/consistent-mask. qla2xxx 0000:1f:00.0: Configuring PCI space...
Which tells me that a 32bit DMA mask is being set for dma_set_mask() and pci_set_consistent_dma_mask() since dma_get_required_mask() is returning back 7fffffff -- no upper-dword bits set...
...
Quoted 16 lines
Quoted 2 lines
printing out all the warnings will actually slow down the system.
Another reason is that there is a lot of code that still doesn't check the return values and when that happens you might get data corruption too.
Quoted 2 lines
Andi, what do you propose as a solution?
A different interface; like I wrote in my earlier mail.
Another probabibility would be to have a blocking interface to swiotlb that won't fail. That would be the better solution long term, but i was told it is hard to fit into some current driver interfaces.
Quoted 3 lines · Andrew Vasquez
yes, the card can support 64bit DMA transfers. but in this case the 'required' DMA mask returned from
dma_get_required_mask()states that a 32bit mask would suffice.
That's a bug in the kernel then that needs to be fixed. Find out why it does that and change it. Don't fix the symptoms, fix the root cause.
-Andi
Quoted 7 lines · Andi Kleen
Quoted 3 lines · Andrew Vasquez
yes, the card can support 64bit DMA transfers. but in this case the 'required' DMA mask returned from
dma_get_required_mask()states that a 32bit mask would suffice.That's a bug in the kernel then that needs to be fixed. Find out why it does that and change it. Don't fix the symptoms, fix the root cause.
The system in question only has 2GB of memory.
Why would it need to do 64 bit DMA?
Signature · 3 lines
Politics is the struggle between those who want to make their country the best in the world, and those who believe it already is. Each group calls the other unpatriotic.
Quoted 6 lines · Rik van Riel
Quoted 5 lines · Andi Kleen
Quoted 3 lines · Andrew Vasquez
yes, the card can support 64bit DMA transfers. but in this case the 'required' DMA mask returned from
dma_get_required_mask()states that a 32bit mask would suffice.
Quoted 6 lines
Quoted 2 lines
That's a bug in the kernel then that needs to be fixed. Find out why it does that and change it. Don't fix the symptoms, fix the root cause.
The system in question only has 2GB of memory.
Why would it need to do 64 bit DMA?
It doesn't. But it definitely doesn't need swiotlb either.
Normally swiotlb doesn't even try to bounce when dma mask is <= end_pfn so something must be very wrong in your kernel. It definitely isn't a mainline kernel. If this happens in Xen then Xen just needs fixing -- it should not try to bounce when the normal kernel wouldn't.
-Andi
Quoted 5 lines · Andi Kleen
Normally swiotlb doesn't even try to bounce when dma mask is <=
end_pfnso something must be very wrong in your kernel. It definitely isn't a mainline kernel. If this happens in Xen then Xen just needs fixing -- it should not try to bounce when the normal kernel wouldn't.
Xen needs to bounce when the requested buffer is not contiguous in machine memory (and indeed uses swiotlb for that).
Cheers,
Muli
Quoted 10 lines · Muli Ben-Yehuda
Quoted 5 lines · Andi Kleen
Normally swiotlb doesn't even try to bounce when dma mask is <=
end_pfnso something must be very wrong in your kernel. It definitely isn't a mainline kernel. If this happens in Xen then Xen just needs fixing -- it should not try to bounce when the normal kernel wouldn't.Xen needs to bounce when the requested buffer is not contiguous in machine memory (and indeed uses swiotlb for that).
Then it should just restrict the sg list merging at the block layer to never merge into anything larger than a page. Then this cannot happen or only very rarely.
-Andi