Re: [RESEND PATCH v5 1/2] RDMA/rxe: Support RDMA Atomic Write operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 06, 2022 at 03:53:36PM +0800, Yang, Xiao/杨 晓 wrote:
> On 2022/9/24 6:22, Jason Gunthorpe wrote:
> > On Fri, Jul 08, 2022 at 04:02:36AM +0000, yangx.jy@xxxxxxxxxxx wrote:
> > > +static enum resp_states atomic_write_reply(struct rxe_qp *qp,
> > > +					   struct rxe_pkt_info *pkt)
> > > +{
> > > +	u64 src, *dst;
> > > +	struct resp_res *res = qp->resp.res;
> > > +	struct rxe_mr *mr = qp->resp.mr;
> > > +	int payload = payload_size(pkt);
> > > +
> > > +	if (!res) {
> > > +		res = rxe_prepare_res(qp, pkt, RXE_ATOMIC_WRITE_MASK);
> > > +		qp->resp.res = res;
> > > +	}
> > > +
> > > +	if (!res->replay) {
> > > +#ifdef CONFIG_64BIT
> > > +		memcpy(&src, payload_addr(pkt), payload);
> > > +
> > > +		dst = iova_to_vaddr(mr, qp->resp.va + qp->resp.offset, payload);
> > > +		/* check vaddr is 8 bytes aligned. */
> > > +		if (!dst || (uintptr_t)dst & 7)
> > > +			return RESPST_ERR_MISALIGNED_ATOMIC;
> > > +
> > > +		/* Do atomic write after all prior operations have completed */
> > > +		smp_store_release(dst, src);
> > 
> > Someone needs to fix iova_to_vaddr to do the missing kmap, we can't
> > just assume you can cast a u64 pfn to a vaddr like this.
> 
> Hi Jason,
> 
> Sorry, it is still not clear to me after looking into the related code
> again.
> 
> IMO, SoftRoCE depends on INFINIBAND_VIRT_DMA Kconfig which only allows
> !HIGHMEM so that SoftRoCE can call page_address() to gain a kernel virtual
> address for a page allocated on low memory zone. If a page is allocated on
> high memory zone, we need to gain a kernel virtual address by
> kmap()/kmap_atomic(). Did I miss something? I wonder why it is necessary to
> call kmap()?

People have been thinking of new uses for kmap, rxe still should be
calling it.

The above just explains why it doesn't fail today, it doesn't excuse
the wrong usage.

Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux