Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230110091356.1524-1-cuiyunhui@bytedance.com>
 <CANn89i+Wh2krOy4YFWvBsEx-s_JgQ0HixHAVJwGw18dVPeyiqw@mail.gmail.com> <20230110104419.67294691@gandalf.local.home>
In-Reply-To: <20230110104419.67294691@gandalf.local.home>
From:   =?UTF-8?B?6L+Q6L6J5bSU?= <cuiyunhui@bytedance.com>
Date:   Wed, 11 Jan 2023 11:53:24 +0800
Message-ID: <CAEEQ3w=aU3siD-ubhPB3+Wv10ARfUeR=cUHmvdEp2q+y105vAw@mail.gmail.com>
Subject: Re: [External] Re: [PATCH v5] sock: add tracepoint for send recv length
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Eric Dumazet <edumazet@google.com>, mhiramat@kernel.org,
        davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
        kuniyu@amazon.com, xiyou.wangcong@gmail.com,
        duanxiongchun@bytedance.com, linux-kernel@vger.kernel.org,
        linux-trace-kernel@vger.kernel.org, netdev@vger.kernel.org,
        dust.li@linux.alibaba.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Jan 10, 2023 at 11:44 PM Steven Rostedt <rostedt@goodmis.org> wrote=
:
>
>
>         TP_printk("sk address =3D %p, family =3D %s protocol =3D %s, leng=
th =3D %d, error =3D %d, flags =3D 0x%x",
>                   __entry->sk, show_family_name(__entry->family),
>                   show_inet_protocol_name(__entry->protocol),
>                   __entry->ret > 0 ? ret : 0, __entry->ret < 0 ? ret : 0,
>                   __entry->flags)
> );
>
> DEFINE_EVENT(sock_msg_length, sock_send_length,
>         TP_PROTO(struct sock *sk, int ret, int flags),
>
>         TP_ARGS(sk, ret, flags)
> );
>
> DEFINE_EVENT_PRINT(sock_msg_length, sock_recv_length,
>         TP_PROTO(struct sock *sk, int ret, int flags),
>
>         TP_ARGS(sk, ret, flags)
>
>         TP_printk("sk address =3D %p, family =3D %s protocol =3D %s, leng=
th =3D %d, error =3D %d, flags =3D 0x%x",
>                   __entry->sk, show_family_name(__entry->family),
>                   show_inet_protocol_name(__entry->protocol),
>                   !(__entry->flags & MSG_PEEK) ? __entry->ret : __entry->=
ret > 0 ? ret : 0,
>                   __entry->ret < 0 ? ret : 0,
>                   __entry->flags)
> );
> #endif /* _TRACE_SOCK_H */
>
> As DEFINE_EVENT_PRINT() uses the class template, but overrides the
> TP_printk() portion (still saving memory).
>

Hi Steve, Based on your suggestion, can we use the following code
instead of using DEFINE_EVENT_PRINT =EF=BC=9F

DECLARE_EVENT_CLASS(sock_msg_length,

        TP_PROTO(struct sock *sk, int ret, int flags),

        TP_ARGS(sk, ret, flags),

        TP_STRUCT__entry(
                __field(void *, sk)
                __field(__u16, family)
                __field(__u16, protocol)
                __field(int, ret)
                __field(int, flags)
        ),

        TP_fast_assign(
                __entry->sk =3D sk;
                __entry->family =3D sk->sk_family;
                __entry->protocol =3D sk->sk_protocol;
                __entry->ret =3D ret;
                __entry->flags =3D flags;
        ),

        TP_printk("sk address =3D %p, family =3D %s protocol =3D %s, length
=3D %d, error =3D %d, flags =3D 0x%x",
                  __entry->sk, show_family_name(__entry->family),
                  show_inet_protocol_name(__entry->protocol),
                  !(__entry->flags & MSG_PEEK) ?
                  (__entry->ret > 0 ? __entry->ret : 0) : 0,
                  __entry->ret < 0 ? __entry->ret : 0,
                  __entry->flags)
);

DEFINE_EVENT(sock_msg_length, sock_send_length,
        TP_PROTO(struct sock *sk, int ret, int flags),

        TP_ARGS(sk, ret, flags)
);

DEFINE_EVENT(sock_msg_length, sock_recv_length,
        TP_PROTO(struct sock *sk, int ret, int flags),

        TP_ARGS(sk, ret, flags)
);


> And then both calls can just do:
>
>         trace_sock_send_length(sk, ret, 0);
>
>         trace_sock_recv_length(sock->sk, ret, flags);
>
> And I bet that will also solve all the gcc being smart waste.
>
> -- Steve
>

Btw,  we still need noinline helpers, right?
Otherwise the following code would be inlined into sock_recvmsg:
mov    0x18(%r13),%rsi
mov    %gs:0x7e832d87(%rip),%eax        # 0x2e68c <pcpu_hot+12>
mov    %eax,%eax
bt     %rax,0xdca591(%rip)        # 0xffffffff825c5ea0 <__cpu_online_mask>


Thanks,
Yunhui