From 4a854137ac9c793bfc09fa4a46bcc6ad938f2fb7 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 15 Feb 2010 16:22:56 -0600 Subject: [PATCH] Allow GetSomeSpace_r to select an optimal host Previously GetSomeSpace_r would never find an 'ideal' host for which to clear callbacks, since lih0_r and lih1_r required a non-NULL rock to do anything. Remove the requirement for the passed-in host rock to be non-NULL, and make lih*_r more threadsafe, by passing in a parameter struct for the rock. Also attempt to make the GSS_r code a bit more clear with some descriptive variable names and such. FIXES 126451 Reviewed-on: http://gerrit.openafs.org/1317 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear (cherry picked from commit eb7d90fd0db68ea49ec1820adda599b5922c7de2) Change-Id: I1c283153977f47cb21c57bf6e65305068c6fd3be Reviewed-on: http://gerrit.openafs.org/1370 Tested-by: Andrew Deason Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear (cherry picked from commit f3899ac3b342ad065dd9484720a4d721a3fb549a) --- src/viced/callback.c | 142 ++++++++++++++++++++++++++++--------------- 1 file changed, 93 insertions(+), 49 deletions(-) diff --git a/src/viced/callback.c b/src/viced/callback.c index 4d0cfd51c..198e08c21 100644 --- a/src/viced/callback.c +++ b/src/viced/callback.c @@ -1575,51 +1575,88 @@ CleanupTimedOutCallBacks_r(void) return (ntimedout > 0); } -static struct host *lih_host; -static int lih_host_held; +/** + * parameters to pass to lih*_r from h_Enumerate_r when trying to find a host + * from which to clear callbacks. + */ +struct lih_params { + /** + * Points to the least interesting host found; try to clear callbacks on + * this host after h_Enumerate_r(lih*_r)'ing. + */ + struct host *lih; + + /** + * The last host we got from lih*_r, but we couldn't clear its callbacks + * for some reason. Choose the next-best host after this one (with the + * current lih*_r, this means to only select hosts that have an ActiveCall + * newer than lastlih). + */ + struct host *lastlih; + + /** + * 1 if lih was held, 0 if it was not and should be released. + */ + int lih_held; +}; /* This version does not allow 'host' to be selected unless its ActiveCall - * is newer than 'hostp' which is the host with the oldest ActiveCall from - * the last pass (if it is provided). We filter out any hosts that are - * are held by other threads. + * is newer than 'params->lastlih' which is the host with the oldest + * ActiveCall from the last pass (if it is provided). We filter out any hosts + * that are are held by other threads. + * + * There is a small problem here, but it may not be easily fixable. Say we + * select some host A, and give it back to GetSomeSpace_r. GSS_r for some + * reason cannot clear the callbacks on A, and so calls us again with + * lastlih = A. Suppose there is another host B that has the same ActiveCall + * time as A. We will now skip over host B, since + * 'hostB->ActiveCall > hostA->ActiveCall' is not true. This could result in + * us prematurely going to the GSS_r 2nd or 3rd pass, and making us a little + * inefficient. This should be pretty rare, though, except perhaps in cases + * with very small numbers of hosts. + * + * Also filter out any hosts with HOSTDELETED set. h_Enumerate_r should in + * theory not give these to us anyway, but be paranoid. */ static int -lih0_r(register struct host *host, register int held, - register struct host *hostp) +lih0_r(struct host *host, int held, void *rock) { + struct lih_params *params = (struct lih_params *)rock; + if (host->cblist - && (hostp && host != hostp) + && (!(host->hostFlags & HOSTDELETED)) && (!held && !h_OtherHolds_r(host)) - && (!lih_host || host->ActiveCall < lih_host->ActiveCall) - && (!hostp || host->ActiveCall > hostp->ActiveCall)) { - if (lih_host != NULL && lih_host_held) { - h_Release_r(lih_host); + && (!params->lih || host->ActiveCall < params->lih->ActiveCall) + && (!params->lastlih || host->ActiveCall > params->lastlih->ActiveCall)) { + + if (params->lih && !params->lih_held) { + h_Release_r(params->lih); /* release prev host */ } - lih_host = host; - lih_host_held = !held; + + params->lih = host; + params->lih_held = held; held = 1; } return held; } -/* This version does not allow 'host' to be selected unless its ActiveCall - * is newer than 'hostp' which is the host with the oldest ActiveCall from - * the last pass (if it is provided). In this second varient, we do not - * prevent held hosts from being selected. - */ +/* same as lih0_r, except we do not prevent held hosts from being selected. */ static int -lih1_r(register struct host *host, register int held, - register struct host *hostp) +lih1_r(struct host *host, int held, void *rock) { + struct lih_params *params = (struct lih_params *)rock; + if (host->cblist - && (hostp && host != hostp) - && (!lih_host || host->ActiveCall < lih_host->ActiveCall) - && (!hostp || host->ActiveCall > hostp->ActiveCall)) { - if (lih_host != NULL && lih_host_held) { - h_Release_r(lih_host); + && (!(host->hostFlags & HOSTDELETED)) + && (!params->lih || host->ActiveCall < params->lih->ActiveCall) + && (!params->lastlih || host->ActiveCall > params->lastlih->ActiveCall)) { + + if (params->lih && !params->lih_held) { + h_Release_r(params->lih); /* release prev host */ } - lih_host = host; - lih_host_held = !held; + + params->lih = host; + params->lih_held = held; held = 1; } return held; @@ -1640,8 +1677,10 @@ extern struct host *hostList; static int GetSomeSpace_r(struct host *hostp, int locked) { - register struct host *hp, *hp1, *hp2; + struct host *hp; + struct lih_params params; int i = 0; + int lastlih_held = 1; cbstuff.GotSomeSpaces++; ViceLog(5, @@ -1652,38 +1691,43 @@ GetSomeSpace_r(struct host *hostp, int locked) } i = 0; - hp1 = NULL; - hp2 = hostList; + params.lastlih = NULL; + do { - lih_host = 0; - h_Enumerate_r(i == 0 ? lih0_r : lih1_r, hp2, (char *)hp1); - hp = lih_host; + params.lih = NULL; + + h_Enumerate_r(i == 0 ? lih0_r : lih1_r, hostList, ¶ms); + + hp = params.lih; + if (params.lastlih && !lastlih_held) { + h_Release_r(params.lastlih); + lastlih_held = 1; + } + params.lastlih = NULL; + if (hp) { - /* set in lih_r! private copy before giving up H_LOCK */ - int lih_host_held2=lih_host_held; + /* note that 'hp' was held by lih*_r; we will need to release it */ cbstuff.GSS4++; if ((hp != hostp) && !ClearHostCallbacks_r(hp, 0 /* not locked or held */ )) { - if (lih_host_held2) + if (!params.lih_held) { h_Release_r(hp); + } return 0; } - if (lih_host_held2) { - h_Release_r(hp); - hp = NULL; - } - hp1 = hp; - hp2 = hostList; + + params.lastlih = hp; + lastlih_held = params.lih_held; + /* params.lastlih will be released on the next iteration, after + * h_Enumerate_r */ + } else { /* * Next time try getting callbacks from any host even if - * it's deleted (that's actually great since we can freely - * remove its callbacks) or it's held since the only other - * option is starvation for the file server (i.e. until the - * callback timeout arrives). + * it's held, since the only other option is starvation for + * the file server (i.e. until the callback timeout arrives). */ i++; - hp1 = NULL; - hp2 = hostList; + params.lastlih = NULL; cbstuff.GSS1++; ViceLog(5, ("GSS: Try harder for longest inactive host cnt= %d\n", -- 2.39.5