From 843d705ca6f0250c3760ec2aa1f3403d19de3df1 Mon Sep 17 00:00:00 2001 From: Rainer Toebbicke Date: Mon, 6 Dec 2010 15:39:25 +0100 Subject: [PATCH] Atomically collect callbacks to be broken Collect callbacks to be broken in one go, otherwise a file server may lock out a client while new callbacks tickle in. With revised multi_Rx, responses get handled early, taking away an argument for issuing callbacks in small chunks -> crank up chunk size. Change-Id: I6822256715d1388aa1a44049315813ea08009105 Reviewed-on: http://gerrit.openafs.org/3909 Reviewed-by: Derrick Brashear Tested-by: BuildBot Reviewed-by: Jeffrey Altman --- src/viced/callback.c | 40 +++++++++++++++++++++++++--------------- src/viced/callback.h | 3 ++- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/viced/callback.c b/src/viced/callback.c index 073d563239..13b5cf17f9 100644 --- a/src/viced/callback.c +++ b/src/viced/callback.c @@ -799,8 +799,8 @@ BreakCallBack(struct host *xhost, AFSFid * fid, int flag) { struct FileEntry *fe; struct CallBack *cb, *nextcb; - struct cbstruct cba[MAX_CB_HOSTS]; - int ncbas; + struct cbstruct cbaDef[MAX_CB_HOSTS], *cba = cbaDef; + unsigned int ncbas, cbaAlloc = MAX_CB_HOSTS; struct AFSCBFids tf; int hostindex; char hoststr[16]; @@ -825,8 +825,7 @@ BreakCallBack(struct host *xhost, AFSFid * fid, int flag) tf.AFSCBFids_len = 1; tf.AFSCBFids_val = fid; - for (; cb;) { - for (ncbas = 0; cb && ncbas < MAX_CB_HOSTS; cb = nextcb) { + for (ncbas = 0; cb ; cb = nextcb) { nextcb = itocb(cb->cnext); if ((cb->hhead != hostindex || flag) && (cb->status == CB_BULK || cb->status == CB_NORMAL @@ -843,6 +842,21 @@ BreakCallBack(struct host *xhost, AFSFid * fid, int flag) } else { if (!(thishost->hostFlags & HOSTDELETED)) { h_Hold_r(thishost); + if (ncbas == cbaAlloc) { /* Need more space */ + int curLen = cbaAlloc*sizeof(cba[0]); + struct cbstruct *cbaOld = (cba == cbaDef) ? NULL : cba; + + /* There are logical contraints elsewhere that the number of hosts + (i.e. h_HTSPERBLOCK*h_MAXHOSTTABLES) remains in the realm of a signed "int". + cbaAlloc is defined unsigned int hence doubling below cannot overflow + */ + cbaAlloc = cbaAlloc<<1; /* double */ + cba = realloc(cbaOld, cbaAlloc * sizeof(cba[0])); + + if (cbaOld == NULL) { /* realloc wouldn't have copied from cbaDef */ + memcpy(cba, cbaDef, curLen); + } + } cba[ncbas].hp = thishost; cba[ncbas].thead = cb->thead; ncbas++; @@ -856,20 +870,16 @@ BreakCallBack(struct host *xhost, AFSFid * fid, int flag) } if (ncbas) { - MultiBreakCallBack_r(cba, ncbas, &tf, xhost); + struct cbstruct *cba2; + int num; - /* we need to to all these initializations again because MultiBreakCallBack may block */ - fe = FindFE(fid); - if (!fe) { - goto done; - } - cb = itocb(fe->firstcb); - if (!cb || ((fe->ncbs == 1) && (cb->hhead == hostindex) && !flag)) { - /* the most common case is what follows the || */ - goto done; + for (cba2 = cba, num = ncbas; ncbas > 0; cba2 += num, ncbas -= num) { + num = (ncbas > MAX_CB_HOSTS) ? MAX_CB_HOSTS : ncbas; + MultiBreakCallBack_r(cba2, num, &tf, xhost); } } - } + + if (cba != cbaDef) free(cba); done: H_UNLOCK; diff --git a/src/viced/callback.h b/src/viced/callback.h index 5359de45d3..58e6ee0d41 100644 --- a/src/viced/callback.h +++ b/src/viced/callback.h @@ -19,7 +19,8 @@ * thereby swamping the file server. As a result, something like * 10 or 15 might be a better bet. */ -#define MAX_CB_HOSTS 10 +/* With revised multi_Rx, responses get handled as early as they tickle in, so try big */ +#define MAX_CB_HOSTS 1024 /* max time to break a callback, otherwise client is dead or net is hosed */ #define MAXCBT 25