On Tue, Jun 11, 2024 at 11:26:12AM GMT, Nhat Pham wrote: [...] > > Hmmm yeah in the past, I set it to NULL to make sure we're not > replacing zswap_next_shrink with an offlined memcg, after that zswap > offlining callback for that memcg has been completed.. > > I suppose we can just call mem_cgroup_iter(...) on that offlined > cgroup, but I'm not 100% sure what happens when we call this function > on a cgroup that is currently being offlined, and has gone past the > zswap offline callback stage. So I was just playing it safe and > restart from the top of the tree :) > > I think this implementation has that behavior right? We see that the > memcg is offlined, so we drop the lock and go to the beginning of the > loop. We reacquire the lock, and might see that zswap_next_shrink == > memcg, so we call mem_cgroup_iter(...) on it. Is this safe? > > Note that zswap_shrink_lock only orders serializes this memcg > selection loop with memcg offlining after it - there's no guarantee > what's the behavior is for memcg offlining before it (well other than > one reference that we manage to acquire thanks to > mem_cgroup_iter(...), so that memcg has not been freed, but not sure > what we can guarantee regarding its place in the memcg hierarchy > tree?). > > Johannes, do you have any idea what we can expect here? Let me also cc Shakeel. > > > mem_cgroup_iter() does a pre-order traversal, so you can see mixture of online and offlined memcgs during a traversal.