Re: [PATCH] powerpc/pseries/iommu: switch to Default DMA window during kdump

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Hi Gaurav,

Thanks for the patch. Few observation, inputs and queries -

Gaurav Batra <gbatra@linux.ibm.com> writes:

> In PowerPC (pseries) a non-virtualized adapter will have 2 DMA windows -
> 2GB default and a larger Dynamic DMA Window (DDW). DDW is large enough to
> map total RAM to a device.
>
> During normal functioning of OS, since RAM is pre-mapped, 2GB default
> window is not used. The only scenario it might get used is when buffers in
> pmemory are mapped to the device for DMA.
>
> As of today, during kdump, during early device discovery, pci_dma_find()
> finds that the device has 2 DMA windows. It selects to use DDW. This is a
> kdump path and DMA window is needed for IO to the device.
>
> Since, in the previous life of the LPAR (before panic), RAM was pre-mapped
> via DDW, the DDW is completely full. So, in the iommu table initialization
> code, iommu_table_clear() frees KDUMP_MIN_TCE_ENTRIES (2K) number of
> TCEs.
>
> But, it seems these are not enough for NVMe over Fibre-channel. When kdump
> is trying to save vmcore on storage device, which is NVMe-FC, the
> TCE usage is much more than 2K number of entries. The driver is mapping
> a lot more buffers for DMA. After all the TCEs are consumed, iommu returns
> iommu_alloc failures and the driver is not able to further map buffers for
> IO. kdump fails to copy vmcore to NVMe-FC storage device.

Some context I collected while reviewing this patch - 

1. kdump environment generally prefers configurations so that we could
   avoid issues like memory allocation failures. I guess, we don't want to
   be running the system with max configurations - that is also the reason
   why distros keep nr_cpus to a lower value during kdump case.
   For e.g. see this [1]:
        scsi: lpfc: Limit xri count for kdump environment

        scsi-mq operation inherently performs pre-allocation of resources for
        blk-mq request queues. Even though the kdump environment reduces the
        configuration to a single CPU, thus 1 hardware queue, which helps
        significantly, the resources are still rather large due to the per request
        allocations.
        <...>
   [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=31f06d2e73726160645f8d9976a0b3f42e136da7

2. While looking more into this call stack path I also saw -
            /*
            * If a crashdump is active, then we are potentially in a very
        -	 * memory constrained environment. Limit us to 1 queue and
        -	 * 64 tags to prevent using too much memory.
        +	 * memory constrained environment. Limit us to  64 tags to prevent
        +	 * using too much memory.
            */
        -	if (is_kdump_kernel()) {
        -		set->nr_hw_queues = 1;
        -		set->nr_maps = 1;
        +	if (is_kdump_kernel())
                set->queue_depth = min(64U, set->queue_depth);
        -	}
        +
        @@ -4515,7 +4513,7 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
                                GFP_KERNEL, set->numa_node);
                if (!set->map[i].mq_map)
                    goto out_free_mq_map;
        -		set->map[i].nr_queues = is_kdump_kernel() ? 1 : set->nr_hw_queues;
        +		set->map[i].nr_queues = set->nr_hw_queues;
            }

            blk_mq_update_queue_map(set); 

   So looks like we already reduce blk-mq queue_depth to 64 in
   blk_mq_alloc_tag_set(), no matter what the queue_depth is passed to
   us by the driver.

3. Also with above patch from v6.9 onwards, we made nr_queues as
   set->nr_hw_queues, whereas earlier it was clamped to 1. Because the
   code assumes that in kdump kernel we boot with nr_cpus=1..
        blk-mq: don't change nr_hw_queues and nr_maps for kdump kernel

        For most of ARCHs, 'nr_cpus=1' is passed for kdump kernel, so
        nr_hw_queues for each mapping is supposed to be 1 already.

        More importantly, this way may cause trouble for driver, because blk-mq and
        driver see different queue mapping since driver should setup hardware
        queue setting before calling into allocating blk-mq tagset.

        So not overriding nr_hw_queues and nr_maps for kdump kernel.

   [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ec30b461f3d067bd322a6c3a6c5105746ed9bf14


So looking at above, I had few questions -
1. In your kdump kernel how many nr_cpus you are booting up with?
   Recently what I heard rhel/sles might be using nr_cpus=16/32. @Sourabh?
   So the calculation in nvme-fc driver then becomes:
     32(nr_cpus) * 64(queue_depth) * 2(cmd+resp) = 4096

  If you are able to reproduce this issue 100% of the time - then can
  you try kdump with nr_cpus=1 and see whether it fixes your iommu alloc
  failure?
2. Also were there more than 1 controller attached? Which can change the
  above calculation then.

Looking at the lpfc and nvme-fc driver - a lot of the calculation are
based on nr_online_cpus. I somehow think if we clamp that value of
nr_online_cpus, we should stop seeing these alloc failures.
Thoughts?

Hopefully, if you can work on some above points further, it will also
explain why are we seeing this failures only now. Is this something that
has caused an issue after RHEL/SLES moved to nr_cpus=16/32?
Or was it after this commit from v6.9?
   ec30b461f3d: ("blk-mq: don't change nr_hw_queues and nr_maps for kdump kernel")

>
> Here are the driver logs and stack
>
> lpfc 0153:70:00.0: iommu_alloc failed,
> tbl 0000000034ebcf5e vaddr 00000000d814df0b npages 1
> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
> lpfc 0153:70:00.0: iommu_alloc failed,
> tbl 0000000034ebcf5e vaddr 000000009779e4d2 npages 1
> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
>
> iommu_map_phys+0x1c4/0x1f0 (unreliable)
> dma_iommu_map_phys+0x54/0xa0
> dma_map_phys+0x3f8/0x590
> __nvme_fc_init_request+0x110/0x300 [nvme_fc]
> nvme_fc_init_request+0x60/0xb8 [nvme_fc]
> blk_mq_alloc_map_and_rqs+0x388/0x510
> blk_mq_alloc_tag_set+0x2a4/0x5f0
> nvme_alloc_io_tag_set+0xe0/0x1e0 [nvme_core]
> nvme_fc_connect_ctrl_work+0x85c/0xdac [nvme_fc]
> process_one_work+0x1e4/0x5a0
> worker_thread+0x1ec/0x3e0
>
> Increasing the number of free TCE entries in iommu_table_clear() will
> increase the probability of hitting EEH since there could still be some
> active IOs from the previous life of the kernel.
>
> Instead, during kdump, we can switch to default 2GB DMA window. This window
> will mostly be empty. Or, could be slightly used if buffers in pmemory
> were mapped for IO.
>

This will still remain a problem when we have SR-IOV adapter attached
correct? Because in that case we only get 1 window, so we anyway can't
use default window in kdump case. Correct?

-ritesh