RE: [EXT] Re: [PATCH] cnic: change __GFP_COMP allocation method

Nilesh Javali
2023-12-19T06:16:59Z

Christoph,

> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Tuesday, December 19, 2023 11:29 AM
> To: Nilesh Javali <njavali@marvell.com>
> Cc: martin.petersen@oracle.com; lduncan@suse.com; cleech@redhat.com;
> linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@marvell.com>; jmeneghi@redhat.com
> Subject: [EXT] Re: [PATCH] cnic: change __GFP_COMP allocation method
>
> External Email
>
> ----------------------------------------------------------------------
> On Tue, Dec 19, 2023 at 11:25:14AM +0530, Nilesh Javali wrote:
> > The dma_alloc_coherent no more provide __GFP_COMP allocation as per
> commit,
> > dma-mapping: reject __GFP_COMP in dma_alloc_attrs, and hence
> > instead use __get_free_pages for __GFP_COMP allocation along with
> > dma_map_single to get dma address in order to fix page reference
> counting
> > issue caused in iscsiuio mmap.
>
> You can't just do a single map for things mapped to userspace, as that
> breaks setups that are not DMA coherent. There was a patch floating
> around to explicitly support dma coherent allocations in uio, which is
> the right thing to do.

If you are referring to the series proposed by Chris Leech, then this had
objections. And that was the reason to look for an alternative method for
coherent DMA mapping.

[PATCH 0/3] UIO_MEM_DMA_COHERENT for cnic/bnx2/bnx2x

During bnx2i iSCSI testing we ran into page refcounting issues in the
uio mmaps exported from cnic to the iscsiuio process, and bisected back
to the removal of the __GFP_COMP flag from dma_alloc_coherent calls.

In order to fix these drivers to be able to mmap dma coherent memory via
a uio device, without resorting to hacks and working with an iommu
enabled, introduce a new uio mmap type backed by dma_mmap_coherent.

While converting the uio interface, I also noticed that not all of these
allocations were PAGE_SIZE aligned. Particularly the bnx2/bnx2x status
block mapping was much smaller than any architecture page size, and I
was concerned that it could be unintentionally exposing kernel memory.

Chris Leech (3):
uio: introduce UIO_DMA_COHERENT type
cnic. bnx2, bnx2x: page align uio mmap allocations
cnic, bnx2, bnx2x: use UIO_MEM_DMA_COHERENT

drivers/net/ethernet/broadcom/bnx2.c | 2 ++
.../net/ethernet/broadcom/bnx2x/bnx2x_main.c | 10 +++---
drivers/net/ethernet/broadcom/cnic.c | 34 ++++++++++++-------
drivers/net/ethernet/broadcom/cnic.h | 1 +
drivers/net/ethernet/broadcom/cnic_if.h | 1 +
drivers/uio/uio.c | 34 +++++++++++++++++++
include/linux/uio_driver.h | 12 +++++--
7 files changed, 75 insertions(+), 19 deletions(-)

Thanks,
Nilesh