On Fri 05-02-21 07:59:06, Shakeel Butt wrote: > +Cc Roman > > On Fri, Feb 5, 2021 at 2:49 AM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > [snip] > > > > > Also, css_get is enough because page > > > > > has a reference to the memcg. > > > > > > > > tryget used to be there to guard against offlined memcg but we have > > > > concluded this is impossible in this path. tryget stayed there to catch > > > > some unexpected cases IIRC. > > > > > > Yeah, it can catch some unexpected cases. But why is this path > > > special so that we need a tryget? > > > > I do not remember details and the changelog of that change is not > > explicit but I suspect it was just because this one could trigger as > > there are external callers to memcg. Is this protection needed? I am not > > sure, this is for you to justify if you want to remove it. > > > > It used to be css_tryget_online() which was changed to css_tryget() > and from the discussion at [1], it seemed css_get() would be enough > but we took a safer route. > > Anyways, I think we can either take the page_memcg_rcu() route or put > explicit restrictions with page lock or lock_page_memcg() to guarantee > page and memcg binding. I don't have a strong opinion either way but I > think removing restrictions in future for new use-cases will be much > harder, so, page_memcg_rcu() approach seems more appropriate at least > for now. Yeah, I would like to not have very special locking requirements here. Definitely not page_lock as that one is too overloaded already. > > [1] https://lore.kernel.org/linux-mm/CALvZod5pAv=u8L2Tgk0hDY7XAiiF2dvjC1omQ5BSfzFu_2zSXA@xxxxxxxxxxxxxx/ -- Michal Hocko SUSE Labs
