lkml.org 
[lkml]   [2022]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH mm v5 0/9] memcg: accounting for objects allocated by mkdir, cgroup
    From
    On 7/1/22 14:03, Michal Hocko wrote:
    > On Mon 27-06-22 09:37:14, Shakeel Butt wrote:
    >> On Fri, Jun 24, 2022 at 6:59 AM Michal Hocko <mhocko@suse.com> wrote:
    > [...]
    >>> Is it even possible to prevent from id
    >>> depletion by the memory consumption? Any medium sized memcg can easily
    >>> consume all the ids AFAICS.
    >>
    >> Though the patch series is pitched as protection against OOMs, I think
    >> it is beneficial irrespective. Protection against an adversarial actor
    >> should not be the aim here. IMO this patch series improves the memory
    >> association to the actual user which is better than unattributed
    >> memory treated as system overhead.
    >
    > Considering the amount of memory and "normal" cgroup usage (I guess we
    > can agree that delegated subtrees do not count their cgroups in
    > thousands) is this really something that is worth bothering with?
    >
    > I mean, these patches are really small and not really disruptive so I do
    > not really see any problem with them. Except that they clearly add a
    > maintenance overhead. Not directly with the memory they track but any
    > future cgroup/memcg metadata related objects would need to be tracked as
    > well and I am worried this will get quickly out of sync. So we will have
    > a half assed solution in place that doesn't really help any containment
    > nor it provides a good and robust consumption tracking.
    >
    > All that being said I find these changes rather without a great value or
    > use.

    Dear Michal,
    I sill have 2 questions:
    1) if you do not want to account any memory allocated for cgroup objects,
    should you perhaps revert commit 3e38e0aaca9e "mm: memcg: charge memcg percpu
    memory to the parent cgroup". Is it an exception perhaps?
    (in fact I hope you will not revert this patch, I just would like to know
    your explanations about this accounting)
    2) my patch set includes kernfs accounting required for proper netdevices accounting

    Allocs Alloc Allocation
    number size
    --------------------------------------------
    1 + 128 (__kernfs_new_node+0x4d) kernfs node
    1 + 88 (__kernfs_iattrs+0x57) kernfs iattrs
    1 + 96 (simple_xattr_alloc+0x28) simple_xattr, can grow over 4Kb
    1 32 (simple_xattr_set+0x59)
    1 8 (__kernfs_new_node+0x30)

    2/9] memcg: enable accounting for kernfs nodes
    3/9] memcg: enable accounting for kernfs iattrs
    4/9] memcg: enable accounting for struct simple_xattr

    What do you think about them? Should I resend them as a new separate patch set?

    Thank you,
    Vasily Averin

    \
     
     \ /
      Last update: 2022-07-10 20:54    [W:4.174 / U:0.296 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site