From mboxrd@z Thu Jan  1 00:00:00 1970
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Subject: Re: [PATCH net-next v2 1/2] udp: msg_zerocopy
Date: Thu, 29 Nov 2018 11:17:57 -0500
Message-ID: <CAF=yD-KZK7R+CFg9CVR6bx3E7Qgk9rEUP0FVt89C74rRnSQahg@mail.gmail.com>
References: <20181126152939.258443-1-willemdebruijn.kernel@gmail.com>
 <20181126152939.258443-2-willemdebruijn.kernel@gmail.com> <11350ff03fc6b03e34f8b4acd063371c887758d8.camel@redhat.com>
 <CAF=yD-J-QP_zUFY9W7CMdjOPgAwkHGgSfVQfVb=EKTNOZB4P8Q@mail.gmail.com>
 <ae1b8e52272b14a49ff4225e647c86c0aab57980.camel@redhat.com>
 <CAF=yD-KG+wyHkX40FXPXNiNEHeKjOd5hPSDFuh-z0pUAnLehLg@mail.gmail.com>
 <CAF=yD-KoUX=suZaaV3xzixSM2Kds65YRxCR4HyXXkKp8=aN+XQ@mail.gmail.com>
 <CAF=yD-L4bBm_obmsW5m7aTGwFv8uC-hDA7zo9qsAUr0y32Rr0Q@mail.gmail.com> <4cd6006ed79a2690ee89f46c699553064f2500f4.camel@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Cc: Network Development <netdev@vger.kernel.org>,
        David Miller <davem@davemloft.net>,
        Willem de Bruijn <willemb@google.com>
To: Paolo Abeni <pabeni@redhat.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-ed1-f68.google.com ([209.85.208.68]:40427 "EHLO
        mail-ed1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1728136AbeK3DY3 (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 29 Nov 2018 22:24:29 -0500
Received: by mail-ed1-f68.google.com with SMTP id d3so2372008edx.7
        for <netdev@vger.kernel.org>; Thu, 29 Nov 2018 08:18:35 -0800 (PST)
In-Reply-To: <4cd6006ed79a2690ee89f46c699553064f2500f4.camel@redhat.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Thu, Nov 29, 2018 at 3:27 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> Thank you for the update!
>
> On Wed, 2018-11-28 at 18:50 -0500, Willem de Bruijn wrote:
> > I did revert to the basic implementation using an extra ref
> > for the function call, similar to TCP, as you suggested.
> >
> > On top of that as a separate optimization patch I have a
> > variant that uses refcnt zero by replacing refcount_inc with
> > refcount_set(.., refcount_read(..) + 1). Not very pretty.
>
> If the skb/uarg is not shared (no other threads can touch the refcnt)
> before ip*_append_data() completes, how about something like the
> following (incremental diff on top of patch 1/2, untested, uncompiled,
> just to give the idea):
>
> The basic idea is using the same schema currently used for wmem
> accounting: do the book-keeping inside the loop and set the atomic
> reference counter only once at the end of the loop.
>
> WDYT?

Thanks for the suggestion. I think that a variant of this might be the
best option indeed.

The common case that we care about (ip_make_skb without
fragmentation) only builds one skb, so we won't necessarily
amortize many atomic ops. I also really would like to avoid the
atomic op and branch in the non-error return path.

But with uarg_refs we can indeed elide the refcount_inc on the
first skb_zcopy_get and detect at skb_zerocopy_put_abort if the
extra reference went unused and it has to put the ref itself.

Let me code that up. Thanks!