Re: [PATCH net-next v5] net/sock: Introduce trace_sk_data_ready()

[Leon Romanovsky]

On Thu, Oct 13, 2022 at 05:00:58PM -0700, Peilin Ye wrote:
> From: Peilin Ye <peilin.ye@xxxxxxxxxxxxx>
> 
> As suggested by Cong, introduce a tracepoint for all ->sk_data_ready()
> callback implementations.  For example:
> 
> <...>
>   ksoftirqd/0-16  [000] ..s..  99.784482: sk_data_ready: family=10 protocol=58 func=sock_def_readable
>   ksoftirqd/0-16  [000] ..s..  99.784819: sk_data_ready: family=10 protocol=58 func=sock_def_readable
> <...>
> 
> Suggested-by: Cong Wang <cong.wang@xxxxxxxxxxxxx>
> Signed-off-by: Peilin Ye <peilin.ye@xxxxxxxxxxxxx>

Please don't reply-to new patches and always send them as new threads
with links to previous versions in changelog.

> ---
> change since v4:
>   - Add back tracepoint in iscsi_target_sk_data_ready()
> 
> changes since v3:
>   - Avoid using __func__ everywhere (Leon Romanovsky)
>   - Delete tracepoint in iscsi_target_sk_data_ready()
> 
> change since v2:
>   - Fix modpost error for modules (kernel test robot)
> 
> changes since v1:
>   - Move tracepoint into ->sk_data_ready() callback implementations
>     (Eric Dumazet)
>   - Fix W=1 warning (Jakub Kicinski)
> 
>  drivers/infiniband/hw/erdma/erdma_cm.c   |  3 +++
>  drivers/infiniband/sw/siw/siw_cm.c       |  5 +++++
>  drivers/infiniband/sw/siw/siw_qp.c       |  3 +++
>  drivers/nvme/host/tcp.c                  |  3 +++
>  drivers/nvme/target/tcp.c                |  5 +++++
>  drivers/scsi/iscsi_tcp.c                 |  3 +++
>  drivers/soc/qcom/qmi_interface.c         |  3 +++
>  drivers/target/iscsi/iscsi_target_nego.c |  2 ++
>  drivers/xen/pvcalls-back.c               |  5 +++++
>  fs/dlm/lowcomms.c                        |  5 +++++
>  fs/ocfs2/cluster/tcp.c                   |  5 +++++
>  include/trace/events/sock.h              | 24 ++++++++++++++++++++++++
>  net/ceph/messenger.c                     |  4 ++++
>  net/core/net-traces.c                    |  2 ++
>  net/core/skmsg.c                         |  3 +++
>  net/core/sock.c                          |  2 ++
>  net/kcm/kcmsock.c                        |  3 +++
>  net/mptcp/subflow.c                      |  3 +++
>  net/qrtr/ns.c                            |  3 +++
>  net/rds/tcp_listen.c                     |  2 ++
>  net/rds/tcp_recv.c                       |  2 ++
>  net/sctp/socket.c                        |  3 +++
>  net/smc/smc_rx.c                         |  3 +++
>  net/sunrpc/svcsock.c                     |  5 +++++
>  net/sunrpc/xprtsock.c                    |  3 +++
>  net/tipc/socket.c                        |  3 +++
>  net/tipc/topsrv.c                        |  5 +++++
>  net/tls/tls_sw.c                         |  3 +++
>  net/xfrm/espintcp.c                      |  3 +++
>  29 files changed, 118 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/erdma/erdma_cm.c b/drivers/infiniband/hw/erdma/erdma_cm.c
> index f13f16479eca..084da6698080 100644
> --- a/drivers/infiniband/hw/erdma/erdma_cm.c
> +++ b/drivers/infiniband/hw/erdma/erdma_cm.c
> @@ -16,6 +16,7 @@
>  #include <linux/types.h>
>  #include <linux/workqueue.h>
>  #include <net/addrconf.h>
> +#include <trace/events/sock.h>
>  
>  #include <rdma/ib_user_verbs.h>
>  #include <rdma/ib_verbs.h>
> @@ -933,6 +934,8 @@ static void erdma_cm_llp_data_ready(struct sock *sk)
>  {
>  	struct erdma_cep *cep;
>  
> +	trace_sk_data_ready(sk);
> +
>  	read_lock(&sk->sk_callback_lock);

I see this pattern in all places and don't know if it is correct or not,
but you are calling to trace_sk_data_ready() at the beginning of
function and do it without taking sk_callback_lock.

Thanks