Re: [PATCH net 1/1] s390/ism: fix receive message buffer allocation

[Gerd Bayer <gbayer@linux.ibm.com>]

On Thu, 2024-03-28 at 16:41 +0100, Gerd Bayer wrote:
> Since [1], dma_alloc_coherent() does not accept requests for GFP_COMP
> anymore, even on archs that may be able to fulfill this.
> Functionality that
> relied on the receive buffer being a compound page broke at that
> point:
> The SMC-D protocol, that utilizes the ism device driver, passes
> receive
> buffers to the splice processor in a struct splice_pipe_desc with a
> single entry list of struct pages. As the buffer is no longer a
> compound
> page, the splice processor now rejects requests to handle more than a
> page worth of data.
> 
> Replace dma_alloc_coherent() and allocate a buffer with kmalloc()
> then
> create a DMA map for it with dma_map_page(). Since only receive
> buffers
> on ISM devices use DMA, qualify the mapping as FROM_DEVICE.
> Since ISM devices are available on arch s390, only and on that arch
> all
> DMA is coherent, there is no need to introduce and export some kind
> of
> dma_sync_to_cpu() method to be called by the SMC-D protocol layer.
> 
> Analogously, replace dma_free_coherent by a two step dma_unmap_page,
> then kfree to free the receive buffer.
> 
> [1] https://lore.kernel.org/all/20221113163535.884299-1-hch@lst.de/
> 
> Fixes: c08004eede4b ("s390/ism: don't pass bogus GFP_ flags to
> dma_alloc_coherent")

Late adding Christoph as the "blamed" committer.

> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> ---
>  drivers/s390/net/ism_drv.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
> index 2c8e964425dc..25911b887e5e 100644
> --- a/drivers/s390/net/ism_drv.c
> +++ b/drivers/s390/net/ism_drv.c
> @@ -14,6 +14,8 @@
>  #include <linux/err.h>
>  #include <linux/ctype.h>
>  #include <linux/processor.h>
> +#include <linux/dma-direction.h>
> +#include <linux/gfp_types.h>
>  
>  #include "ism.h"
>  
> @@ -292,13 +294,15 @@ static int ism_read_local_gid(struct ism_dev
> *ism)
>  static void ism_free_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
>  {
>  	clear_bit(dmb->sba_idx, ism->sba_bitmap);
> -	dma_free_coherent(&ism->pdev->dev, dmb->dmb_len,
> -			  dmb->cpu_addr, dmb->dma_addr);
> +	dma_unmap_page(&ism->pdev->dev, dmb->dma_addr, dmb->dmb_len,
> +		       DMA_FROM_DEVICE);
> +	kfree(dmb->cpu_addr);
>  }
>  
>  static int ism_alloc_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
>  {
>  	unsigned long bit;
> +	int rc;
>  
>  	if (PAGE_ALIGN(dmb->dmb_len) > dma_get_max_seg_size(&ism-
> >pdev->dev))
>  		return -EINVAL;
> @@ -315,14 +319,27 @@ static int ism_alloc_dmb(struct ism_dev *ism,
> struct ism_dmb *dmb)
>  	    test_and_set_bit(dmb->sba_idx, ism->sba_bitmap))
>  		return -EINVAL;
>  
> -	dmb->cpu_addr = dma_alloc_coherent(&ism->pdev->dev, dmb-
> >dmb_len,
> -					   &dmb->dma_addr,
> -					   GFP_KERNEL | __GFP_NOWARN
> |
> -					   __GFP_NOMEMALLOC |
> __GFP_NORETRY);
> -	if (!dmb->cpu_addr)
> -		clear_bit(dmb->sba_idx, ism->sba_bitmap);
> +	dmb->cpu_addr = kmalloc(dmb->dmb_len, GFP_KERNEL |
> __GFP_NOWARN |
> +				__GFP_COMP | __GFP_NOMEMALLOC |
> __GFP_NORETRY);
> +	if (!dmb->cpu_addr) {
> +		rc = -ENOMEM;
> +		goto out_bit;
> +	}
> +	dmb->dma_addr = dma_map_page(&ism->pdev->dev,
> +				     virt_to_page(dmb->cpu_addr), 0,
> +				     dmb->dmb_len, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(&ism->pdev->dev, dmb->dma_addr)) {
> +		rc = -ENOMEM;
> +		goto out_free;
> +	}
> +
> +	return 0;
>  
> -	return dmb->cpu_addr ? 0 : -ENOMEM;
> +out_free:
> +	kfree(dmb->cpu_addr);
> +out_bit:
> +	clear_bit(dmb->sba_idx, ism->sba_bitmap);
> +	return rc;
>  }
>  
>  int ism_register_dmb(struct ism_dev *ism, struct ism_dmb *dmb,