Re: [PATCH net-next v4 11/27] tcp: support externally provided ubufs

[David Ahern <dsahern@kernel.org>]

On 7/8/22 7:03 AM, Pavel Begunkov wrote:
>>> @@ -1356,9 +1362,11 @@ int tcp_sendmsg_locked(struct sock *sk, struct
>>> msghdr *msg, size_t size)
>>>                 copy = min_t(int, copy, pfrag->size - pfrag->offset);
>>>   -            if (tcp_downgrade_zcopy_pure(sk, skb))
>>> -                goto wait_for_space;
>>> -
>>> +            if (unlikely(skb_zcopy_pure(skb) ||
>>> skb_zcopy_managed(skb))) {
>>> +                if (tcp_downgrade_zcopy_pure(sk, skb))
>>> +                    goto wait_for_space;
>>> +                skb_zcopy_downgrade_managed(skb);
>>> +            }
>>>               copy = tcp_wmem_schedule(sk, copy);
>>>               if (!copy)
>>>                   goto wait_for_space;
>>
>> You dropped the msg->msg_ubuf checks on jump labels. Removing the one
>> you had at 'out_nopush' I agree with based on my tests (i.e, it is not
>> needed).
> 
> It was an optimisation, which I dropped for simplicity. Will be sending it
> and couple more afterwards.
> 
> 
>> The one at 'out_err' seems like it is needed - but it has been a few
>> weeks since I debugged that case. I believe the error path I was hitting
>> was sk_stream_wait_memory with MSG_DONTWAIT flag set meaning timeout is
>> 0 and it jumps there with EPIPE.
> 
> Currently, it's consistent with MSG_ZEROCOPY ubuf_info, we grab a ubuf_info
> reference at the beginning (msg_zerocopy_realloc() for MSG_ZEROCOPY and
> net_zcopy_get() for msg_ubuf), and then release it at the end
> with net_zcopy_put() or net_zcopy_put_abort().
> 

my fault; I somehow dropped a line in the port to the 5.13 kernel.