A path forward to cleaning up dying cgroups?

A path forward to cleaning up dying cgroups?

[Hamza Mahfooz]

I was just curious as to what the status of the issue described in [1]
is. It appears that the last time someone took a stab at it was in [2].

Though it seems like there has been relative silence regarding it since
then. So, has there been any discussion regarding the issue since then
and does anyone know if there is consensus on how we should go about
resolving this issue within the kernel?

BR,
Hamza

[1] https://lwn.net/Articles/895431/
[2] https://lore.kernel.org/r/20220621125658.64935-1-songmuchun@bytedance.com/

Re: A path forward to cleaning up dying cgroups?

[Hamza Mahfooz]

Cc: Shakeel Butt <shakeel.butt@linux.dev>

On 2/5/25 12:48, Hamza Mahfooz wrote:
> I was just curious as to what the status of the issue described in [1]
> is. It appears that the last time someone took a stab at it was in [2].
> 
> Though it seems like there has been relative silence regarding it since
> then. So, has there been any discussion regarding the issue since then
> and does anyone know if there is consensus on how we should go about
> resolving this issue within the kernel?
> 
> BR,
> Hamza
> 
> [1] https://lwn.net/Articles/895431/
> [2] https://lore.kernel.org/r/20220621125658.64935-1-songmuchun@bytedance.com/

Re: A path forward to cleaning up dying cgroups?

[Johannes Weiner]

On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
> Cc: Shakeel Butt <shakeel.butt@linux.dev>
> 
> On 2/5/25 12:48, Hamza Mahfooz wrote:
> > I was just curious as to what the status of the issue described in [1]
> > is. It appears that the last time someone took a stab at it was in [2].

If memory serves, the sticking point was whether pages should indeed
be reparented on cgroup death, or whether they could be moved
arbitrarily to other cgroups that are still using them.

It's a bit unfortunate, because the reparenting patches were tested
and reviewed, and the arbitrary recharging was just an idea that
ttbomk nobody seriously followed up on afterwards.

We also recently removed the charge moving code from cgroup1, along
with the subtle page access/locking/accounting rules it imposed on the
rest of the MM. I'm doubtful there is much appetite in either camp for
bringing this back.

So I would still love to see Muchun's patches merged. They fix a
seemingly universally experienced operational issue in memcg, and we
shouldn't hold it up unless somebody actually posts alternative code.

Thoughts?

Re: A path forward to cleaning up dying cgroups?

[Yosry Ahmed]

On Wed, Feb 05, 2025 at 01:08:42PM -0500, Johannes Weiner wrote:
> On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
> > Cc: Shakeel Butt <shakeel.butt@linux.dev>
> > 
> > On 2/5/25 12:48, Hamza Mahfooz wrote:
> > > I was just curious as to what the status of the issue described in [1]
> > > is. It appears that the last time someone took a stab at it was in [2].
> 
> If memory serves, the sticking point was whether pages should indeed
> be reparented on cgroup death, or whether they could be moved
> arbitrarily to other cgroups that are still using them.
> 
> It's a bit unfortunate, because the reparenting patches were tested
> and reviewed, and the arbitrary recharging was just an idea that
> ttbomk nobody seriously followed up on afterwards.

There was an RFC series [1] for the recharging, but all memcg
maintainers hated it :P

https://lore.kernel.org/lkml/20230720070825.992023-1-yosryahmed@google.com/

> 
> We also recently removed the charge moving code from cgroup1, along
> with the subtle page access/locking/accounting rules it imposed on the
> rest of the MM. I'm doubtful there is much appetite in either camp for
> bringing this back.

Yeah with the charge moving code gone the case for recharging grows
weaker.

> 
> So I would still love to see Muchun's patches merged. They fix a
> seemingly universally experienced operational issue in memcg, and we
> shouldn't hold it up unless somebody actually posts alternative code.
> 
> Thoughts?

Adding Zach and Kinsey who were recently looking into this from the
Google side.

Re: A path forward to cleaning up dying cgroups?

[Kairui Song]

On Thu, Feb 6, 2025 at 2:16 AM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
>
> On Wed, Feb 05, 2025 at 01:08:42PM -0500, Johannes Weiner wrote:
> > On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
> > > Cc: Shakeel Butt <shakeel.butt@linux.dev>
> > >
> > > On 2/5/25 12:48, Hamza Mahfooz wrote:
> > > > I was just curious as to what the status of the issue described in [1]
> > > > is. It appears that the last time someone took a stab at it was in [2].
> >
> > If memory serves, the sticking point was whether pages should indeed
> > be reparented on cgroup death, or whether they could be moved
> > arbitrarily to other cgroups that are still using them.
> >
> > It's a bit unfortunate, because the reparenting patches were tested
> > and reviewed, and the arbitrary recharging was just an idea that
> > ttbomk nobody seriously followed up on afterwards.
>
> There was an RFC series [1] for the recharging, but all memcg
> maintainers hated it :P
>
> https://lore.kernel.org/lkml/20230720070825.992023-1-yosryahmed@google.com/

We have been suffering from dying cgroup issues for years too, and I
just saw this series. Will it be a good idea to combine this with
reparenting instead (if we will go with the reparenting approach)?
Using objcg API to charge the folios does help speed up the
reparenting, but also adds some overhead and complexity. Just walking
and reparenting the folios seems a more direct approach.

And another idea is, per our observation, dying cgroups have few pages
that are mapped, as the process has all exited. Most folios are just
cache. Shared mapped pages are minor especially for containers. So a
deferred recharge on access seems good enough? Mapped folios may also
be finally unmap someday and get recharged. And at least this makes
accounting more accurate.

Re: A path forward to cleaning up dying cgroups?

[Roman Gushchin]

On Wed, Feb 05, 2025 at 01:08:42PM -0500, Johannes Weiner wrote:
> On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
> > Cc: Shakeel Butt <shakeel.butt@linux.dev>
> > 
> > On 2/5/25 12:48, Hamza Mahfooz wrote:
> > > I was just curious as to what the status of the issue described in [1]
> > > is. It appears that the last time someone took a stab at it was in [2].
> 
> If memory serves, the sticking point was whether pages should indeed
> be reparented on cgroup death, or whether they could be moved
> arbitrarily to other cgroups that are still using them.
> 
> It's a bit unfortunate, because the reparenting patches were tested
> and reviewed, and the arbitrary recharging was just an idea that
> ttbomk nobody seriously followed up on afterwards.
> 
> We also recently removed the charge moving code from cgroup1, along
> with the subtle page access/locking/accounting rules it imposed on the
> rest of the MM. I'm doubtful there is much appetite in either camp for
> bringing this back.
> 
> So I would still love to see Muchun's patches merged. They fix a
> seemingly universally experienced operational issue in memcg, and we
> shouldn't hold it up unless somebody actually posts alternative code.
> 
> Thoughts?

I don't have a strong opinion here. Reparenting is clearly not perfect,
but I agree that we don't have any better solutions, only vague ideas.
I believe Muchun's code would require some refresh, but generally is fine
to merge.

This all comes up to the handling of memory shared between cgroups.
Sharing can be spatial (2 or more simultaneously existing cgroups) or
temporal (a cgroup is being deleted and recreated, the workload tries to
reuse old pages). The reparenting turns temporal sharing into the spacial.
It helps with dying cgroups, but comes at the cost of permanently wrong
accounting and issues with the memory protection.

Thanks!

Re: A path forward to cleaning up dying cgroups?

[Shakeel Butt]

On Wed, Feb 05, 2025 at 01:08:42PM -0500, Johannes Weiner wrote:
> On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
> > Cc: Shakeel Butt <shakeel.butt@linux.dev>
> > 
> > On 2/5/25 12:48, Hamza Mahfooz wrote:
> > > I was just curious as to what the status of the issue described in [1]
> > > is. It appears that the last time someone took a stab at it was in [2].
> 
> If memory serves, the sticking point was whether pages should indeed
> be reparented on cgroup death, or whether they could be moved
> arbitrarily to other cgroups that are still using them.
> 
> It's a bit unfortunate, because the reparenting patches were tested
> and reviewed, and the arbitrary recharging was just an idea that
> ttbomk nobody seriously followed up on afterwards.
> 
> We also recently removed the charge moving code from cgroup1, along
> with the subtle page access/locking/accounting rules it imposed on the
> rest of the MM. I'm doubtful there is much appetite in either camp for
> bringing this back.
> 
> So I would still love to see Muchun's patches merged. They fix a
> seemingly universally experienced operational issue in memcg, and we
> shouldn't hold it up unless somebody actually posts alternative code.
> 
> Thoughts?

I think the recharging (or whatever the alternative) can be a followup
to this. I agree this is a good change.

Re: A path forward to cleaning up dying cgroups?

[Muchun Song]

> On Feb 6, 2025, at 02:46, Shakeel Butt <shakeel.butt@linux.dev> wrote:
> 
> On Wed, Feb 05, 2025 at 01:08:42PM -0500, Johannes Weiner wrote:
>> On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
>>> Cc: Shakeel Butt <shakeel.butt@linux.dev>
>>> 
>>> On 2/5/25 12:48, Hamza Mahfooz wrote:
>>>> I was just curious as to what the status of the issue described in [1]
>>>> is. It appears that the last time someone took a stab at it was in [2].
>> 
>> If memory serves, the sticking point was whether pages should indeed
>> be reparented on cgroup death, or whether they could be moved
>> arbitrarily to other cgroups that are still using them.
>> 
>> It's a bit unfortunate, because the reparenting patches were tested
>> and reviewed, and the arbitrary recharging was just an idea that
>> ttbomk nobody seriously followed up on afterwards.
>> 
>> We also recently removed the charge moving code from cgroup1, along
>> with the subtle page access/locking/accounting rules it imposed on the
>> rest of the MM. I'm doubtful there is much appetite in either camp for
>> bringing this back.
>> 
>> So I would still love to see Muchun's patches merged. They fix a
>> seemingly universally experienced operational issue in memcg, and we
>> shouldn't hold it up unless somebody actually posts alternative code.
>> 
>> Thoughts?
> 
> I think the recharging (or whatever the alternative) can be a followup
> to this. I agree this is a good change.

I agree with you. We've been encountering dying memory issues for years
on our servers. As Roman said, I need to refresh my patches. So I need
some time for refreshing.

Muchun,
Thanks.

Re: A path forward to cleaning up dying cgroups?

[Waiman Long]

On 2/5/25 10:30 PM, Muchun Song wrote:
>
>> On Feb 6, 2025, at 02:46, Shakeel Butt <shakeel.butt@linux.dev> wrote:
>>
>> On Wed, Feb 05, 2025 at 01:08:42PM -0500, Johannes Weiner wrote:
>>> On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
>>>> Cc: Shakeel Butt <shakeel.butt@linux.dev>
>>>>
>>>> On 2/5/25 12:48, Hamza Mahfooz wrote:
>>>>> I was just curious as to what the status of the issue described in [1]
>>>>> is. It appears that the last time someone took a stab at it was in [2].
>>> If memory serves, the sticking point was whether pages should indeed
>>> be reparented on cgroup death, or whether they could be moved
>>> arbitrarily to other cgroups that are still using them.
>>>
>>> It's a bit unfortunate, because the reparenting patches were tested
>>> and reviewed, and the arbitrary recharging was just an idea that
>>> ttbomk nobody seriously followed up on afterwards.
>>>
>>> We also recently removed the charge moving code from cgroup1, along
>>> with the subtle page access/locking/accounting rules it imposed on the
>>> rest of the MM. I'm doubtful there is much appetite in either camp for
>>> bringing this back.
>>>
>>> So I would still love to see Muchun's patches merged. They fix a
>>> seemingly universally experienced operational issue in memcg, and we
>>> shouldn't hold it up unless somebody actually posts alternative code.
>>>
>>> Thoughts?
>> I think the recharging (or whatever the alternative) can be a followup
>> to this. I agree this is a good change.
> I agree with you. We've been encountering dying memory issues for years
> on our servers. As Roman said, I need to refresh my patches. So I need
> some time for refreshing.

Glad to hear that. I have been waiting for a resolution of the dying 
memory cgroup problems for years :-)

Cheers,
Longman

Re: A path forward to cleaning up dying cgroups?

[Kamalesh Babulal]

On 06/02/25 09:00, Muchun Song wrote:
> 
> 
>> On Feb 6, 2025, at 02:46, Shakeel Butt <shakeel.butt@linux.dev> wrote:
>>
>> On Wed, Feb 05, 2025 at 01:08:42PM -0500, Johannes Weiner wrote:
>>> On Wed, Feb 05, 2025 at 12:50:19PM -0500, Hamza Mahfooz wrote:
>>>> Cc: Shakeel Butt <shakeel.butt@linux.dev>
>>>>
>>>> On 2/5/25 12:48, Hamza Mahfooz wrote:
>>>>> I was just curious as to what the status of the issue described in [1]
>>>>> is. It appears that the last time someone took a stab at it was in [2].
>>>
>>> If memory serves, the sticking point was whether pages should indeed
>>> be reparented on cgroup death, or whether they could be moved
>>> arbitrarily to other cgroups that are still using them.
>>>
>>> It's a bit unfortunate, because the reparenting patches were tested
>>> and reviewed, and the arbitrary recharging was just an idea that
>>> ttbomk nobody seriously followed up on afterwards.
>>>
>>> We also recently removed the charge moving code from cgroup1, along
>>> with the subtle page access/locking/accounting rules it imposed on the
>>> rest of the MM. I'm doubtful there is much appetite in either camp for
>>> bringing this back.
>>>
>>> So I would still love to see Muchun's patches merged. They fix a
>>> seemingly universally experienced operational issue in memcg, and we
>>> shouldn't hold it up unless somebody actually posts alternative code.
>>>
>>> Thoughts?
>>
>> I think the recharging (or whatever the alternative) can be a followup
>> to this. I agree this is a good change.
> 
> I agree with you. We've been encountering dying memory issues for years
> on our servers. As Roman said, I need to refresh my patches. So I need
> some time for refreshing.
> 

We have seen the dying cgroups issue too and look forward to your patches.
Happy to help with testing/reviewing.

-- 
Thanks,
Kamalesh