Hi Dai and all,
first of all, apologies for the long delay on my side.
I am referring to Dai's latest webrev at
http://cr.opensolaris.org/~dain/6817942 :
Prepared by:Dai Ngo on Mon May 4 23:53:10 PDT 2009
> What would you think about a fix following your idea of always putting
> the last used entry upfront the list, but removing the time stamp code?
Dai, in the latest version of your webrev you have removed the prev_time code.
Thanks.
You have also removed the other static declarations, which is good.
Let's find some agreement about which fix to integrate, yours or mine
(http://cr.opensolaris.org/~nigoroll/rpc_loadbalancing_6817942/).
What I do still like about my suggested change is that it reduces complexity
for
dooming entries and that I viewed it as a starting point for introducing a more
scalable data structure for the RPC connection table.
But I really do agree now that your proposed solution is better because it
involves only minimal change and is very simple.
I would like to make one (last?) suggestion, though:
In this piece of code ...
1889 if ((i > 1) && (prev != &cm_hd)) {
1890 /*
1891 * remove and re-insert entry at head of
list.
1892 */
1893 *prev = lru_entry->x_next;
1894 lru_entry->x_next = cm_hd;
1895 cm_hd = lru_entry;
1896 }
... it is assumed that prev has been properly set whenever i > 1. I fear that
future changes could break this assumption.
I would suggest to initialize prev in line 1777 of your CR to make this a
little
more robust.
prev = cmp = &cm_hd;
Other than that, I'm happy with your change now. I also want to state again
publicly that, in retrospect, I think I have not taken your suggestions as
constructively as I would have wanted to. I want to try to be as objective as
possible, and I think that in this case I failed to do so.
Thank you,
Nils
