Re: [PATCH for-next v2 06/18] RDMA/rxe: Add rxe_add_frag() to rxe_mr.c

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Nov 30, 2022 at 06:16:53PM -0600, Bob Pearson wrote:
> On 11/30/22 17:36, Jason Gunthorpe wrote:
> > On Wed, Nov 30, 2022 at 02:53:22PM -0600, Bob Pearson wrote:
> >> On 11/24/22 13:10, Jason Gunthorpe wrote:
> >>> On Mon, Oct 31, 2022 at 03:27:55PM -0500, Bob Pearson wrote:
> >>>> +int rxe_add_frag(struct sk_buff *skb, struct rxe_phys_buf *buf,
> >>>> +		 int length, int offset)
> >>>> +{
> >>>> +	int nr_frags = skb_shinfo(skb)->nr_frags;
> >>>> +	skb_frag_t *frag = &skb_shinfo(skb)->frags[nr_frags];
> >>>> +
> >>>> +	if (nr_frags >= MAX_SKB_FRAGS) {
> >>>> +		pr_debug("%s: nr_frags (%d) >= MAX_SKB_FRAGS\n",
> >>>> +			__func__, nr_frags);
> >>>> +		return -EINVAL;
> >>>> +	}
> >>>> +
> >>>> +	frag->bv_len = length;
> >>>> +	frag->bv_offset = offset;
> >>>> +	frag->bv_page = virt_to_page(buf->addr);
> >>>
> >>> Assuming this is even OK to do, then please do the xarray conversion
> >>> I sketched first:
> >>>
> >>> https://lore.kernel.org/linux-rdma/Y3gvZr6%2FNCii9Avy@nvidia.com/
> >>
> >> I've been looking at this. Seems incorrect for IB_MR_TYPE_DMA which
> >> do not carry a page map but simply convert iova to kernel virtual
> >> addresses.
> > 
> > There is always a struct page involved, even in the kernel case. You
> > can do virt_to_page on kernel addresses

> Agreed but there isn't a page map set up for DMA mr's. You just get the whole kernel
> address space. So the call to rxe_mr_copy_xarray() won't work. There isn't an
> xarray to copy to/from. Much easier to just leave the DMA mr code in place since
> it does what we want very simply. Also you have to treat the DMA mr separately for
> atomic ops.

You mean the all physical memory MR type? It is true, but you still
have to add the kmap and so on. It should be a similar function that
assumes the IOVA is a physical address is a kernel mapped address and
does virt_to_page/etc instead of the xarray loop.

Jason