On Fri, Mar 14, 2025 at 12:35:33PM +0100, Sebastian Andrzej Siewior wrote:
> On 2025-03-14 11:17:28 [+0100], Vlastimil Babka wrote:
> > On 3/14/25 07:15, Shakeel Butt wrote:
> > > Previously we could not call obj_cgroup_put() inside the local lock
> > > because on the put on the last reference, the release function
> > > obj_cgroup_release() may try to re-acquire the local lock. However that
> > > chain has been broken. Now simply do obj_cgroup_put() inside
> > > drain_obj_stock() instead of returning the old objcg.
> > >
> > > Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> >
> > Hm is this really safe? I can see obj_cgroup_release() doing
> > percpu_ref_exit() -> kfree(), do we have guaranteed that allocation won't be
> > also in a kmemcg and recurse?
>
> This was like this until commit
> 5675114623872 ("mm/memcg: protect memcg_stock with a local_lock_t")
>
> at which point the put had to happen outside. This "percpu_ref_exit() ->
> kfree()" was also prior this commit.
Yes, as Sebastian said, this is as safe as before commit 567511462387.
Also the ref->data which is getting kfree()'ed from percpu_ref_exit() is
not a __GFP_ACCOUNT allocation, so can't recurse.
