Re: [PATCH 0/6] Improve handling of GFP flags in the CMA allocator

[Vlastimil Babka <vbabka@suse.cz>]

On 2/18/19 10:07 PM, Gabriel Krisman Bertazi wrote:
> Hi,
> 
> The main goal of this patchset is to solve a deadlock in the CMA
> allocator, which happens because cma_alloc tries to sleep waiting for an
> IO in the GFP_NOIO path.  This issue, which was reported by Gael Portay
> was discussed here:
> 
> https://groups.google.com/a/lists.one-eyed-alien.net/forum/#!topic/usb-storage/BXpAsg-G1us
> 
> My proposed requires reverting the patches that removed the gfp flags
> information from cma_alloc() (patches 1 to 3).  According to the author,
> that parameter was removed because it misleads developers about what
> cma_alloc actually supports. In his specific case he had problems with
> GFP_ZERO.  With that in mind I gave a try at implementing GFP_ZERO in a
> quite trivial way in patch 4.  Finally, patches 5 and 6 attempt to fix
> the issue by avoiding the unecessary serialization done around
> alloc_contig_range.

I haven't checked in detail yet, but for GFP_NOIO, we have
memalloc_noio_save() / memalloc_noio_restore() which adds implicit
GFP_NOIO for the whole call stack. So that could be perhaps used to
avoid adding the gfp flags back to function signatures. Since you are
adding a new test for __GFP_IO in cma_alloc() in patch 6, you would use
e.g. current_gfp_context(GFP_KERNEL) first to add __GFP_NOIO based on
the implicit context. As for the arm64 caller, maybe it already is in
noio context (ideal world), or would add it based on test before calling
dma_alloc_from_contiguous(). There's also some documentation in
Documentation/core-api/gfp_mask-from-fs-io.rst
CCing Michal for opinion since he authored this

> This is my first adventure in the mm subsystem, so I hope I didn't screw
> up something very obvious. I tested this on the workload that was
> deadlocking (arm board, with CMA intensive operations from the GPU and
> USB), as well as some scripting on top of debugfs.  Is there any
> regression test I should be running, which specially applies to the CMA
> code?
> 
> 
> Gabriel Krisman Bertazi (6):
>   Revert "kernel/dma: remove unsupported gfp_mask parameter from
>     dma_alloc_from_contiguous()"
>   Revert "mm/cma: remove unsupported gfp_mask parameter from
>     cma_alloc()"
>   cma: Warn about callers requesting unsupported flags
>   cma: Add support for GFP_ZERO
>   page_isolation: Propagate temporary pageblock isolation error
>   cma: Isolate pageblocks speculatively during allocation
> 
>  arch/arm/mm/dma-mapping.c                  |  5 +--
>  arch/arm64/mm/dma-mapping.c                |  2 +-
>  arch/powerpc/kvm/book3s_hv_builtin.c       |  2 +-
>  arch/xtensa/kernel/pci-dma.c               |  2 +-
>  drivers/iommu/amd_iommu.c                  |  2 +-
>  drivers/iommu/intel-iommu.c                |  3 +-
>  drivers/s390/char/vmcp.c                   |  2 +-
>  drivers/staging/android/ion/ion_cma_heap.c |  2 +-
>  include/linux/cma.h                        |  2 +-
>  include/linux/dma-contiguous.h             |  4 +-
>  kernel/dma/contiguous.c                    |  6 +--
>  kernel/dma/direct.c                        |  3 +-
>  kernel/dma/remap.c                         |  2 +-
>  mm/cma.c                                   | 51 ++++++++++++++++++----
>  mm/cma_debug.c                             |  2 +-
>  mm/page_isolation.c                        | 20 ++++++---
>  16 files changed, 74 insertions(+), 36 deletions(-)
>