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@xxxxxxxxxx/
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.
This breaks in the mr_copy routine and atomic ops in responder.
There is also a missing rxe_mr_iova_to_index() function. Looks simple enough
just the number of pages starting from 0.
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?
I am happy to go in this direction and if we do it should be ahead of the other
outstanding changes that touch MRs.
I will try to submit a patch to do that.
Bob
>
> And this operation is basically a xa_for_each loop taking 'struct page
> *' off of the MR's xarray, slicing it, then stuffing into the
> skb. Don't call virt_to_page()
>
> *However* I have no idea if it is even safe to stuff unstable pages
> into a skb. Are there other examples doing this? Eg zero copy tcp?
>
> Jason
