On Thu 14-12-23 13:22:29, Dan Schatzberg wrote: > On Thu, Dec 14, 2023 at 09:38:55AM +0100, Michal Hocko wrote: [...] > > While the review can point those out it is quite easy to break and you > > will only learn about that very indirectly. I think it would be easier > > to review and maintain if you go with a pointer that would fallback to > > mem_cgroup_swappiness() if NULL which will be the case for every > > existing reclaimer except memory.reclaim with swappiness value. > > I agree. My initial implementation used a pointer for this > reason. I'll switch this back. Just to be clear - I still need to > initialize scan_control.swappiness in all these other places right? No. They will just get initialized to 0. All you need to make sure is that the swappiness is used consistently. I would propose something like scan_control_swappiness() (or sc_swappiness) which would actually do if (sc->swappiness) return sc->swappiness; return mem_cgroup_swappiness(sc->target_mem_cgroup); and then make sure that mem_cgroup_swappiness is never used directly. -- Michal Hocko SUSE Labs