From b60754ed1c1f2d1593ffc35a5febf39a50404134 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 7 Jun 2021 18:23:39 -0500 Subject: [PATCH] afs: Avoid creating unused conns In afs_LoopServers and PCallBackAddr, we loop over our known servers to issue an RPC, but we skip over servers on various conditions. For most of those, we skip over the server before creating the relevant conn (via afs_ConnBySA), but not for the SRVADDR_ISDOWN/afs_HaveCallBacksFrom check. If we skip over the server for that reason, we'll create an afs_conn and rx_connection, and then release our refs without using them. This doesn't cause any big problems, but it does create extra connections and peers that linger around for at least a couple of hours (until they get cleaned up by afs_GCUserData). It's very easy to avoid these extra useless conns; just perform the SRVADDR_ISDOWN/afs_HaveCallBacksFrom check before we call afs_ConnBYSA. This also makes the check a bit more consistent with the other checks we do. Change-Id: Ie49b87e5dd755f77364502528a02c14944e8c23d Reviewed-on: https://gerrit.openafs.org/14637 Tested-by: BuildBot Reviewed-by: Benjamin Kaduk --- src/afs/afs_pioctl.c | 20 ++++++++++++-------- src/afs/afs_server.c | 27 +++++++++++++++------------ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/afs/afs_pioctl.c b/src/afs/afs_pioctl.c index 2634879..4a705cc 100644 --- a/src/afs/afs_pioctl.c +++ b/src/afs/afs_pioctl.c @@ -5239,6 +5239,11 @@ DECL_PIOCTL(PCallBackAddr) if (!ts->cell) /* not really an active server, anyway, it must */ continue; /* have just been added by setsprefs */ + if ((sa->sa_flags & SRVADDR_ISDOWN) == 0 && !afs_HaveCallBacksFrom(ts)) { + /* Server is up, and we have no active callbacks from it. */ + continue; + } + /* get a connection, even if host is down; bumps conn ref count */ tu = afs_GetUser(areq->uid, ts->cell->cellNum, SHARED_LOCK); tc = afs_ConnBySA(sa, ts->cell->fsport, ts->cell->cellNum, tu, @@ -5247,18 +5252,17 @@ DECL_PIOCTL(PCallBackAddr) if (!tc) continue; - if ((sa->sa_flags & SRVADDR_ISDOWN) || afs_HaveCallBacksFrom(ts)) { - if (sa->sa_flags & SRVADDR_ISDOWN) { - rx_SetConnDeadTime(rxconn, 3); - } + if (sa->sa_flags & SRVADDR_ISDOWN) { + rx_SetConnDeadTime(rxconn, 3); + } #ifdef RX_ENABLE_LOCKS - AFS_GUNLOCK(); + AFS_GUNLOCK(); #endif /* RX_ENABLE_LOCKS */ - code = RXAFS_CallBackRxConnAddr(rxconn, &addr); + code = RXAFS_CallBackRxConnAddr(rxconn, &addr); #ifdef RX_ENABLE_LOCKS - AFS_GLOCK(); + AFS_GLOCK(); #endif /* RX_ENABLE_LOCKS */ - } + afs_PutConn(tc, rxconn, SHARED_LOCK); /* done with it now */ } /* Outer loop over addrs */ afs_osi_Free(addrs, srvAddrCount * sizeof(*addrs)); diff --git a/src/afs/afs_server.c b/src/afs/afs_server.c index 24d4cd3..d4893b1 100644 --- a/src/afs/afs_server.c +++ b/src/afs/afs_server.c @@ -707,6 +707,11 @@ afs_LoopServers(int adown, struct cell *acellp, int vlalso, if (!ts->cell) /* not really an active server, anyway, it must */ continue; /* have just been added by setsprefs */ + if ((sa->sa_flags & SRVADDR_ISDOWN) == 0 && !afs_HaveCallBacksFrom(sa->server)) { + /* Server is up, and we have no active callbacks from it. */ + continue; + } + /* get a connection, even if host is down; bumps conn ref count */ tu = afs_GetUser(treq->uid, ts->cell->cellNum, SHARED_LOCK); tc = afs_ConnBySA(sa, ts->cell->fsport, ts->cell->cellNum, tu, @@ -716,18 +721,16 @@ afs_LoopServers(int adown, struct cell *acellp, int vlalso, if (!tc) continue; - if ((sa->sa_flags & SRVADDR_ISDOWN) || afs_HaveCallBacksFrom(sa->server)) { - conns[nconns]=tc; - rxconns[nconns]=rxconn; - if (sa->sa_flags & SRVADDR_ISDOWN) { - rx_SetConnDeadTime(rxconn, 3); - conntimer[nconns]=1; - } else { - conntimer[nconns]=0; - } - nconns++; - } else /* not holding, kill ref */ - afs_PutConn(tc, rxconn, SHARED_LOCK); + conns[nconns]=tc; + rxconns[nconns]=rxconn; + if (sa->sa_flags & SRVADDR_ISDOWN) { + rx_SetConnDeadTime(rxconn, 3); + conntimer[nconns]=1; + } else { + conntimer[nconns]=0; + } + nconns++; + } /* Outer loop over addrs */ afs_osi_Free(addrs, srvAddrCount * sizeof(*addrs)); -- 1.9.4