> Quoting Roland Dreier <[EMAIL PROTECTED]>:
> 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
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
