On Mon, Nov 12, 2018 at 10:05:11PM -0800, Bjorn Helgaas wrote: > On Tue, Dec 13, 2016 at 06:18:40PM -0500, Keith Busch wrote: > > It's the MSI-x masking that's our next highest contributor. Masking > > vectors still requires non-posted commands, and since they're not > > going through managed API accessors like config space uses, the > > removed flag is needed for checking before doing significant MMIO. > > Sorry for digging up this ancient history, but do you remember where > this MSI-X masking with non-posted commands happens? The masking is > an MMIO write (posted) and there should only be a non-posted MMIO read > if we use the msi_set_mask_bit() path. > > I'm looking at this path: > > nvme_pci_disable > pci_free_irq_vectors > pci_disable_msix > pci_msix_shutdown > if (pci_dev_is_disconnected()) # added by 0170591bb067 > return # (skip hw access) > for_each_pci_msi_entry(...) # <-- loop > __pci_msix_desc_mask_irq > writel # <-- MMIO write > pci_read_config_word(PCI_MSIX_FLAGS) > pci_write_config_word(PCI_MSIX_FLAGS) > free_msi_irqs > > whih only does MMIO *writes*, which I think are posted and do not > require completion and should not cause timeouts. That's wasted > effort, I agree, but it doesn't seem like it should be a performance > issue. > > So there must be another path, probably preceding this one (since > pci_disable_msix() cleans everything up), that masks the vectors and > does the non-posted reads? > > The only places we do MMIO reads are msix_program_entries(), which is > done at MSI-X enable time, and msi_set_mask_bit(), which is used in > irq_chip.irq_mask() and irq_chip.irq_unmask() methods. But I haven't > figured out where that irq_chip path is used in a loop. I must have thought masking was a read-modify-write operation. It looks like we're using a cached value so only posted commands needed.
