| Message ID | 168415757392.9504.14836685251349712202.stgit@manet.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Socket creation observability | expand |
On Mon, 2023-05-15 at 09:32 -0400, Chuck Lever wrote: > From: Chuck Lever <chuck.lever@oracle.com> > > The -ENOMEM arm could fire repeatedly if the system runs low on > memory, so remove it. > > Don't bother to trace -EAGAIN error events, since those fire after > a listener is created (with no work done) and once again after an > accept has been handled successfully (again, with no work done). > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > net/sunrpc/svcsock.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c > index e0fb65e90af2..2058641ab9f6 100644 > --- a/net/sunrpc/svcsock.c > +++ b/net/sunrpc/svcsock.c > @@ -885,13 +885,8 @@ static struct svc_xprt *svc_tcp_accept(struct svc_xprt *xprt) > clear_bit(XPT_CONN, &svsk->sk_xprt.xpt_flags); > err = kernel_accept(sock, &newsock, O_NONBLOCK); > if (err < 0) { > - if (err == -ENOMEM) > - printk(KERN_WARNING "%s: no more sockets!\n", > - serv->sv_name); > - else if (err != -EAGAIN) > - net_warn_ratelimited("%s: accept failed (err %d)!\n", > - serv->sv_name, -err); > - trace_svcsock_accept_err(xprt, serv->sv_name, err); > + if (err != -EAGAIN) > + trace_svcsock_accept_err(xprt, serv->sv_name, err); Would this be better done as a TP_CONDITION tracepoint? > return NULL; > } > set_bit(XPT_CONN, &svsk->sk_xprt.xpt_flags); > >
> On May 15, 2023, at 12:54 PM, Jeff Layton <jlayton@kernel.org> wrote: > > On Mon, 2023-05-15 at 09:32 -0400, Chuck Lever wrote: >> From: Chuck Lever <chuck.lever@oracle.com> >> >> The -ENOMEM arm could fire repeatedly if the system runs low on >> memory, so remove it. >> >> Don't bother to trace -EAGAIN error events, since those fire after >> a listener is created (with no work done) and once again after an >> accept has been handled successfully (again, with no work done). >> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >> --- >> net/sunrpc/svcsock.c | 9 ++------- >> 1 file changed, 2 insertions(+), 7 deletions(-) >> >> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c >> index e0fb65e90af2..2058641ab9f6 100644 >> --- a/net/sunrpc/svcsock.c >> +++ b/net/sunrpc/svcsock.c >> @@ -885,13 +885,8 @@ static struct svc_xprt *svc_tcp_accept(struct svc_xprt *xprt) >> clear_bit(XPT_CONN, &svsk->sk_xprt.xpt_flags); >> err = kernel_accept(sock, &newsock, O_NONBLOCK); >> if (err < 0) { >> - if (err == -ENOMEM) >> - printk(KERN_WARNING "%s: no more sockets!\n", >> - serv->sv_name); >> - else if (err != -EAGAIN) >> - net_warn_ratelimited("%s: accept failed (err %d)!\n", >> - serv->sv_name, -err); >> - trace_svcsock_accept_err(xprt, serv->sv_name, err); >> + if (err != -EAGAIN) >> + trace_svcsock_accept_err(xprt, serv->sv_name, err); > > Would this be better done as a TP_CONDITION tracepoint? I looked at doing that. svcsock_accept_err is in a declared class. I would need to split that into two trace events, one CONDITIONAL and the other not. svc_tcp_accept() is not a hot path, so it really isn't worth a whole lot of churn. Let me know if there's some benefit aside from a bit of code cleanliness that I forgot. >> return NULL; >> } >> set_bit(XPT_CONN, &svsk->sk_xprt.xpt_flags); >> >> > > -- > Jeff Layton <jlayton@kernel.org> -- Chuck Lever
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index e0fb65e90af2..2058641ab9f6 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -885,13 +885,8 @@ static struct svc_xprt *svc_tcp_accept(struct svc_xprt *xprt) clear_bit(XPT_CONN, &svsk->sk_xprt.xpt_flags); err = kernel_accept(sock, &newsock, O_NONBLOCK); if (err < 0) { - if (err == -ENOMEM) - printk(KERN_WARNING "%s: no more sockets!\n", - serv->sv_name); - else if (err != -EAGAIN) - net_warn_ratelimited("%s: accept failed (err %d)!\n", - serv->sv_name, -err); - trace_svcsock_accept_err(xprt, serv->sv_name, err); + if (err != -EAGAIN) + trace_svcsock_accept_err(xprt, serv->sv_name, err); return NULL; } set_bit(XPT_CONN, &svsk->sk_xprt.xpt_flags);