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

Bob Pearson
2022-12-01T00:19:30Z

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.

Bob
>
>> I am curious what the benefit of the 'advanced' API for xarrays buys here. Is it just
>> preallocating all the memory before it gets used?
>
> It runs quite a bit faster
>
> Jason