Messages in this thread | | | From | Eric Dumazet <> | Date | Mon, 9 Jan 2023 15:54:38 +0100 | Subject | Re: [External] Re: [PATCH v4] sock: add tracepoint for send recv length |
| |
On Mon, Jan 9, 2023 at 2:13 PM 运辉崔 <cuiyunhui@bytedance.com> wrote: > > On Mon, Jan 9, 2023 at 5:56 PM Eric Dumazet <edumazet@google.com> wrote: > > > > > Note: At least for CONFIG_RETPOLINE=y and gcc 12.2, compiler adds many > > additional instructions (and additional memory reads), > > even when the trace point is not enabled. > > > > Contrary to some belief, adding a tracepoint is not always 'free'. > > tail calls for example are replaced with normal calls. > > > > > > .popsection > > > > # 0 "" 2 > > #NO_APP > > .L106: > > # net/socket.c:1008: } > > movl %ebx, %eax # <retval>, > > popq %rbx # > > popq %rbp # > > popq %r12 # > > ret > > .L111: > > # ./include/trace/events/sock.h:308: DEFINE_EVENT(sock_msg_length, > > sock_recv_length, > > > > Hi Eric, Thanks for your reply, In fact, it is because the > definition of the tracepoint function is inline, > Not just these two tracepoints,right? > > #define __DECLARE_TRACE(name, proto, args, cond, data_proto) \ > ... > static inline void trace_##name(proto) > > Regarding the above issue, I plan to optimize it like this: > > static noinline void call_trace_sock_send_length(struct sock *sk, __u16 family, > __u16 protocol, int ret, int flags) > { > trace_sock_send_length(sk, family, protocol, ret, 0); > } > > static inline int sock_sendmsg_nosec(struct socket *sock, struct msghdr *msg) > { > int ret = INDIRECT_CALL_INET(sock->ops->sendmsg, inet6_sendmsg, > inet_sendmsg, sock, msg, > msg_data_left(msg)); > BUG_ON(ret == -EIOCBQUEUED); > > if (trace_sock_send_length_enabled()) {
A barrier() is needed here, with the current state of affairs.
IMO, ftrace/x86 experts should take care of this generic issue ?
> call_trace_sock_send_length(sock->sk, sock->sk->sk_family, > sock->sk->sk_protocol, ret, 0); > } > return ret; > } > > What do you think? > > Thanks, > Yunhui
| |