It's pretty clear from the results and from the test coverage to date that Linux wants this. I do think additional review is needed. For the usual code quality reasons, but also to help others get up to speed with the proposed changes and to ensure that we have something which is well maintainable going forward. The code at present is unnecessarily difficult to review and that review will be less effective than it might be, because of its lack of commentary. I'll merge the v10 series into -mm and -next for additional runtime exposure, but I'll be asking for a broad update. The data structures appear to be adequately documented (this is rare) but the code itself is lacking. Please go through each and every new function and ask "is this function so self-evident that it can be presented to a newcomer with no explanatory comments at all". If "yes" then check again. If still "yes" then fine. If "no" then let's craft comments which tell the reader things which are not evident from the code itself. Things which are helpful to that reader. Design concepts, side-effects, preconditions, unobvious traps, etc. Please ensure that the current group of mglru developers review these comment additions as closely as they review code changes. Alas, it's late in the process to be adding these things. But it can be made better. I'll make some high-level comments on the patches themselves now, but I see little point in attempting detailed review when a better-explained version is hopefully forthcoming. A few gratuitous notebook entries: - lru_gen_struct.timestamps works in jiffies? Why? jiffies can vary based on platform and config - why add inter-platform and inter-Kconfig variability like this? Time is measured in seconds! And the amount of work which can be performed in one second varies by, guess, 100x on machines which we care about. This also has potential to introduce tremendous inter-platform variability in the behaviour of this feature. - did lru_gen_use_mm() really need to test nodes_full()? Is that likely enough to be a net benefit? - seq_is_valid() sounds like it belongs to the seq_file() subsystem - does get_nr_evictable() need to return and use signed types (long)? It sums the contents of lru_gen_struct.nr_pages[][][], which is ulong, so I think "no".
