On 4/21/23 13:31, Yang Yang wrote: >> I'm late but FWIW, not supper happy that while the updated calculations are >> now accurate wrt the actual code, the explanation (which was written at the >> time of page cache-only workinset) was more easier to follow in the simpler >> form. Now it's still mostly talking about page cache and explaining the >>balance between its active and inactive list only, and then suddenly the >> anon lists appear out of nowhere in the final equations. > > Thanks for your reviewing! I should update the whole parts. Sorry for > hadn't do it better, please drop the patch, I will try to submit patchv3 > to fix this. I think it's too late to drop, and not worth rebasing the git tree in early merge window for that. >> In other words, I think it would have been better to leave that explanation >> as it was, and then add a new part describing the extension to anon pages. > > I read the description of the source file again carefully, and think that > there is no need to creat a new part, if we explain at the begining that > the word 'pages' include page cache and anonymous page, and do some minor > adjustments. For example: > Per node, two kinds of clock lists are maintained for pages.. I think it would make following the explanation more complicated, and it's already difficult enough. Reasoning just about active vs inactive file list is much simpler to follow the equations and observations behind them. Wonder what Johannes as the original author thinks, anyway.
