[openib-general] [PATCH 3 of 4] IB/mthca: fix non-cache-coherent CPUs with memfree

[Michael S. Tsirkin]

> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCH 3 of 4] IB/mthca: fix non-cache-coherent CPUs with memfree
> 
> OK, I already merged this but now I'm thinking it's somewhat buggy:

Hopefully not.

>  > +		if (coherent)
>  > +			ret = mthca_alloc_icm_coherent(&dev->pdev->dev,
>  > +						       &chunk->mem[chunk->npages],
>  > +						       cur_order, gfp_mask);
>  > +		else
>  > +		       	ret = mthca_alloc_icm_pages(&chunk->mem[chunk->npages],
>  > +						    cur_order, gfp_mask);
>  >  
>  > -			if (++chunk->npages == MTHCA_ICM_CHUNK_LEN) {
>  > +		if (!ret) {
>  > +			++chunk->npages;
>  > +
>  > +			if (!coherent && chunk->npages == MTHCA_ICM_CHUNK_LEN) {
>  >  				chunk->nsg = pci_map_sg(dev->pdev, chunk->mem,
> 
> I don't see anything that ever bumps chunk->nsg if we're allocating a
> coherent region and we end up needing more than one allocation to do
> it.

Yes but this is intentional.

> Maybe something like this on top of the patch?
> 
> diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
> index 0b9d053..48f7c65 100644
> --- a/drivers/infiniband/hw/mthca/mthca_memfree.c
> +++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
> @@ -175,7 +175,9 @@ struct mthca_icm *mthca_alloc_icm(struct mthca_dev *dev, int npages,
>  		if (!ret) {
>  			++chunk->npages;
>  
> -			if (!coherent && chunk->npages == MTHCA_ICM_CHUNK_LEN) {
> +			if (coherent)
> +				++chunk->nsg;
> +			else if (chunk->npages == MTHCA_ICM_CHUNK_LEN) {
>  				chunk->nsg = pci_map_sg(dev->pdev, chunk->mem,
>  							chunk->npages,
>  							PCI_DMA_BIDIRECTIONAL);

No, I think the code is fine and this patch will break things:
chunk->nsg is needed only for non-coherent memory to call pci_unmap_sg:

               if (chunk->nsg > 0)
			pci_unmap_sg(dev->pdev, chunk->mem,
				chunk->npages, PCI_DMA_BIDIRECTIONAL);



and we do *not* want to call pci_unmap_sg on consistent memory.

-- 
MST