From 3376bc13b8722cfb4435458cd8a0131121a7b026 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 12 Aug 2013 17:37:29 -0500 Subject: [PATCH] viced: Avoid endless BCB loop Without this commit, when we break callbacks for a fid, we loop over all callbacks for the fid, break a few of them, and then start over. We do this repeatedly until we run out of callbacks. If a client sees a callback break, and then establishes a new callback promise while the fileserver is still breaking callbacks, the fileserver can break the same callback for the same host again and again. This can continue forever, if the client establishes its new callback promises quickly enough. So to avoid this, when we start breaking callbacks, flag all of the callback structures that we want to look at. Then when we repeatedly loop through all of the callbacks for the fid, only look at the flagged callback structures. This adds a 'flags' field to struct CallBack, and defines a single flag, CBFLAG_BREAKING. This is an alternative fix to the issue also fixed in 843d705c. This implementation avoids allocating extra memory under locks, and has the slight benefit of not breaking callbacks that were elsewhere deleted during the BCB. This comes at the cost of a single extra traversal through our callback list, and the cost of claiming one of the bits in the CallBack structure. Reviewed-on: http://gerrit.openafs.org/10172 Tested-by: BuildBot Reviewed-by: Benjamin Kaduk Reviewed-by: Derrick Brashear (cherry picked from commit 47124f337b43f8731bfbe3bd71e42d046a4d1075) Change-Id: I522e0cecd0a9a10bf9eafaae669f4f0005ced893 Reviewed-on: http://gerrit.openafs.org/10755 Reviewed-by: Andrew Deason Reviewed-by: Benjamin Kaduk Tested-by: BuildBot Reviewed-by: D Brashear Reviewed-by: Stephan Wiesand --- src/viced/callback.c | 25 ++++++++++++++++++++++--- src/viced/callback.h | 6 +++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/viced/callback.c b/src/viced/callback.c index 5566c8105..9daf5362a 100644 --- a/src/viced/callback.c +++ b/src/viced/callback.c @@ -639,6 +639,7 @@ AddCallBack1_r(struct host *host, AFSFid * fid, afs_uint32 * thead, int type, cb->cnext = 0; cb->fhead = fetoi(fe); cb->status = type; + cb->flags = 0; HAdd(cb, host); TAdd(cb, Thead); } @@ -837,13 +838,31 @@ BreakCallBack(struct host *xhost, AFSFid * fid, int flag) tf.AFSCBFids_len = 1; tf.AFSCBFids_val = fid; + /* Set CBFLAG_BREAKING flag on all CBs we're looking at. We do this so we + * can loop through all relevant CBs while dropping H_LOCK, and not lose + * track of which CBs we want to look at. If we look at all CBs over and + * over again, we can loop indefinitely as new CBs are added. */ + for (; cb; cb = nextcb) { + nextcb = itocb(cb->cnext); + + if ((cb->hhead != hostindex || flag) + && (cb->status == CB_BULK || cb->status == CB_NORMAL + || cb->status == CB_VOLUME)) { + cb->flags |= CBFLAG_BREAKING; + } + } + + cb = itocb(fe->firstcb); + osi_Assert(cb); + + /* loop through all CBs, only looking at ones with the CBFLAG_BREAKING + * flag set */ for (; cb;) { for (ncbas = 0; cb && ncbas < MAX_CB_HOSTS; cb = nextcb) { nextcb = itocb(cb->cnext); - if ((cb->hhead != hostindex || flag) - && (cb->status == CB_BULK || cb->status == CB_NORMAL - || cb->status == CB_VOLUME)) { + if ((cb->flags & CBFLAG_BREAKING)) { struct host *thishost = h_itoh(cb->hhead); + cb->flags &= ~CBFLAG_BREAKING; if (!thishost) { ViceLog(0, ("BCB: BOGUS! cb->hhead is NULL!\n")); } else if (thishost->hostFlags & VENUSDOWN) { diff --git a/src/viced/callback.h b/src/viced/callback.h index 5359de45d..a632c39fc 100644 --- a/src/viced/callback.h +++ b/src/viced/callback.h @@ -66,7 +66,8 @@ struct CallBack { afs_uint32 fhead; /* index of associated FE */ u_byte thead; /* Head of timeout chain */ u_byte status; /* Call back status; see definitions, below */ - unsigned short spare; /* ensure proper alignment */ + u_byte flags; /* see CBFLAG_* definitions below */ + u_byte spare; /* ensure proper alignment */ afs_uint32 hhead; /* Head of host table chain */ afs_uint32 tprev, tnext; /* per-timeout circular list of callbacks */ afs_uint32 hprev, hnext; /* per-host circular list of callbacks */ @@ -98,6 +99,9 @@ struct VCBParams { #define CB_VOLUME 3 /* Callback for a volume */ #define CB_BULK 4 /* Normal callbacks, handed out from FetchBulkStatus */ +/* values for the 'flags' field of CallBack structure */ +#define CBFLAG_BREAKING 0x1 /* this CB is marked for breaking / is getting broken */ + /* call back indices to pointers, and vice-versa */ #define itocb(i) ((i)?CB+(i):0) #define cbtoi(cbp) ((afs_uint32)(!(cbp)?0:(cbp)-CB)) -- 2.39.5