The overall approach looks fine to me, although I'm not the arbiter of
taste for the DMA API.
However, I think this wants to be split into at least three parts for
merging: adding the dmaflush flags stuff into the DMA API; adding the
dmaflush parameter to the ib_umem_get() API (and fixing every caller);
and using that API to fix mthca on SGI boxes. (And also there should
probably be a 4th patch to fix the same issue with mlx4 at least, and
possibly further patches for other drivers such as the cxgb3 RDMA
driver, etc)
Also further comments below:
> struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> - size_t size, int access)
> + size_t size, int access, int dmaflush)
This change means every caller of ib_umem_get needs to be fixed up as
part of the patch.
> @@ -1027,7 +1029,14 @@ static struct ib_mr *mthca_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> if (!mr)
> return ERR_PTR(-ENOMEM);
>
> - mr->umem = ib_umem_get(pd->uobject->context, start, length, acc);
> + if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd)) {
> + err = -EFAULT;
> + goto err;
> + }
trivial, but you might as well do the copy_from_udata before
allocating mr, so that you can just return ERR_PTR(-EFAULT) without
having to free mr on the error path.
> + dmaflush = (int) ucmd.mr_attrs & MTHCA_MR_DMAFLUSH;
> +
> + mr->umem = ib_umem_get(pd->uobject->context, start, length, acc,
> + dmaflush);
I think the dmaflush temporary variable isn't adding anything, and
certainly the cast to int is unnecessary. Anyway, it looks better to
me to write this simply as
mr->umem = ib_umem_get(pd->uobject->context, start, length, acc,
ucmd.mr_attrs & MTHCA_MR_DMAFLUSH);
> +#define MTHCA_MR_DMAFLUSH 0x1 /* flush in-flight DMA on a write to
> + * memory region (IA64_SGI_SN2 only) */
I would leave out the commentary about this being SN2-only -- the
whole point is to use a generic API to hide this detail.
- R.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
