From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> To: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org, kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>, Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>, Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org> Subject: Re: [PATCH v5 2/2] decrement static keys on real destroy time Date: Mon, 14 May 2012 11:12:50 -0700 [thread overview] Message-ID: <20120514181250.GD2366@google.com> (raw) In-Reply-To: <1336767077-25351-3-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> On Fri, May 11, 2012 at 05:11:17PM -0300, Glauber Costa wrote: > We call the destroy function when a cgroup starts to be removed, > such as by a rmdir event. > > However, because of our reference counters, some objects are still > inflight. Right now, we are decrementing the static_keys at destroy() > time, meaning that if we get rid of the last static_key reference, > some objects will still have charges, but the code to properly > uncharge them won't be run. > > This becomes a problem specially if it is ever enabled again, because > now new charges will be added to the staled charges making keeping > it pretty much impossible. > > We just need to be careful with the static branch activation: > since there is no particular preferred order of their activation, > we need to make sure that we only start using it after all > call sites are active. This is achieved by having a per-memcg > flag that is only updated after static_key_slow_inc() returns. > At this time, we are sure all sites are active. > > This is made per-memcg, not global, for a reason: > it also has the effect of making socket accounting more > consistent. The first memcg to be limited will trigger static_key() > activation, therefore, accounting. But all the others will then be > accounted no matter what. After this patch, only limited memcgs > will have its sockets accounted. > > [v2: changed a tcp limited flag for a generic proto limited flag ] > [v3: update the current active flag only after the static_key update ] > [v4: disarm_static_keys() inside free_work ] > [v5: got rid of tcp_limit_mutex, now in the static_key interface ] > > Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> > CC: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > CC: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org> > CC: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> > CC: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org> Generally looks sane to me. Please feel free to addmy Reviewed-by. > + if (val == RESOURCE_MAX) > + cg_proto->active = false; > + else if (val != RESOURCE_MAX) { Minor nitpick: CodingStyle says not to omit { } if other branches need them. Thanks. -- tejun
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org> To: Glauber Costa <glommer@parallels.com> Cc: cgroups@vger.kernel.org, linux-mm@kvack.org, devel@openvz.org, kamezawa.hiroyu@jp.fujitsu.com, netdev@vger.kernel.org, Li Zefan <lizefan@huawei.com>, Johannes Weiner <hannes@cmpxchg.org>, Michal Hocko <mhocko@suse.cz> Subject: Re: [PATCH v5 2/2] decrement static keys on real destroy time Date: Mon, 14 May 2012 11:12:50 -0700 [thread overview] Message-ID: <20120514181250.GD2366@google.com> (raw) In-Reply-To: <1336767077-25351-3-git-send-email-glommer@parallels.com> On Fri, May 11, 2012 at 05:11:17PM -0300, Glauber Costa wrote: > We call the destroy function when a cgroup starts to be removed, > such as by a rmdir event. > > However, because of our reference counters, some objects are still > inflight. Right now, we are decrementing the static_keys at destroy() > time, meaning that if we get rid of the last static_key reference, > some objects will still have charges, but the code to properly > uncharge them won't be run. > > This becomes a problem specially if it is ever enabled again, because > now new charges will be added to the staled charges making keeping > it pretty much impossible. > > We just need to be careful with the static branch activation: > since there is no particular preferred order of their activation, > we need to make sure that we only start using it after all > call sites are active. This is achieved by having a per-memcg > flag that is only updated after static_key_slow_inc() returns. > At this time, we are sure all sites are active. > > This is made per-memcg, not global, for a reason: > it also has the effect of making socket accounting more > consistent. The first memcg to be limited will trigger static_key() > activation, therefore, accounting. But all the others will then be > accounted no matter what. After this patch, only limited memcgs > will have its sockets accounted. > > [v2: changed a tcp limited flag for a generic proto limited flag ] > [v3: update the current active flag only after the static_key update ] > [v4: disarm_static_keys() inside free_work ] > [v5: got rid of tcp_limit_mutex, now in the static_key interface ] > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Tejun Heo <tj@kernel.org> > CC: Li Zefan <lizefan@huawei.com> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Michal Hocko <mhocko@suse.cz> Generally looks sane to me. Please feel free to addmy Reviewed-by. > + if (val == RESOURCE_MAX) > + cg_proto->active = false; > + else if (val != RESOURCE_MAX) { Minor nitpick: CodingStyle says not to omit { } if other branches need them. Thanks. -- tejun -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2012-05-14 18:12 UTC|newest] Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-05-11 20:11 [PATCH v5 0/2] fix static_key disabling problem in memcg Glauber Costa 2012-05-11 20:11 ` Glauber Costa 2012-05-11 20:11 ` [PATCH v5 1/2] Always free struct memcg through schedule_work() Glauber Costa 2012-05-11 20:11 ` Glauber Costa 2012-05-11 20:11 ` Glauber Costa [not found] ` <1336767077-25351-2-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> 2012-05-14 0:56 ` KAMEZAWA Hiroyuki 2012-05-14 0:56 ` KAMEZAWA Hiroyuki 2012-05-11 20:11 ` [PATCH v5 2/2] decrement static keys on real destroy time Glauber Costa 2012-05-11 20:11 ` Glauber Costa 2012-05-11 20:11 ` Glauber Costa [not found] ` <1336767077-25351-3-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> 2012-05-14 0:59 ` KAMEZAWA Hiroyuki 2012-05-14 0:59 ` KAMEZAWA Hiroyuki [not found] ` <4FB058D8.6060707-+CUm20s59erQFUHtdCDX3A@public.gmane.org> 2012-05-16 6:03 ` Glauber Costa 2012-05-16 6:03 ` Glauber Costa 2012-05-16 6:03 ` Glauber Costa [not found] ` <4FB3431C.3050402-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> 2012-05-16 7:04 ` Glauber Costa 2012-05-16 7:04 ` Glauber Costa 2012-05-16 7:04 ` Glauber Costa [not found] ` <4FB3518B.3090205-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> 2012-05-16 8:28 ` KAMEZAWA Hiroyuki 2012-05-16 8:28 ` KAMEZAWA Hiroyuki [not found] ` <4FB3652D.2040909-+CUm20s59erQFUHtdCDX3A@public.gmane.org> 2012-05-16 8:30 ` Glauber Costa 2012-05-16 8:30 ` Glauber Costa 2012-05-16 8:30 ` Glauber Costa 2012-05-16 8:37 ` Glauber Costa 2012-05-16 8:37 ` Glauber Costa 2012-05-16 8:37 ` Glauber Costa 2012-05-14 1:38 ` Li Zefan 2012-05-14 1:38 ` Li Zefan [not found] ` <4FB0621C.3010604-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 2012-05-16 7:03 ` Glauber Costa 2012-05-16 7:03 ` Glauber Costa 2012-05-16 7:03 ` Glauber Costa 2012-05-16 20:57 ` Andrew Morton 2012-05-16 20:57 ` Andrew Morton 2012-05-14 18:12 ` Tejun Heo [this message] 2012-05-14 18:12 ` Tejun Heo 2012-05-16 21:06 ` Andrew Morton 2012-05-16 21:06 ` Andrew Morton 2012-05-16 21:06 ` Andrew Morton 2012-05-17 3:06 ` Glauber Costa 2012-05-17 3:06 ` Glauber Costa 2012-05-17 3:06 ` Glauber Costa [not found] ` <4FB46B4C.3000307-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> 2012-05-17 5:37 ` Andrew Morton 2012-05-17 5:37 ` Andrew Morton 2012-05-17 5:37 ` Andrew Morton [not found] ` <20120516223715.5d1b4385.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 2012-05-17 9:52 ` Glauber Costa 2012-05-17 9:52 ` Glauber Costa 2012-05-17 9:52 ` Glauber Costa 2012-05-17 10:18 ` KAMEZAWA Hiroyuki [not found] ` <4FB4D061.10406-+CUm20s59erQFUHtdCDX3A@public.gmane.org> 2012-05-17 10:22 ` Glauber Costa 2012-05-17 10:22 ` Glauber Costa 2012-05-17 10:22 ` Glauber Costa [not found] ` <4FB4D14D.4020303-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> 2012-05-17 10:27 ` KAMEZAWA Hiroyuki 2012-05-17 10:27 ` KAMEZAWA Hiroyuki 2012-05-17 15:19 ` Tejun Heo 2012-05-17 17:02 ` Andrew Morton 2012-05-17 17:02 ` Andrew Morton 2012-05-16 21:13 ` Andrew Morton 2012-05-16 21:13 ` Andrew Morton [not found] ` <20120516141342.911931e7.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> 2012-05-17 0:07 ` KAMEZAWA Hiroyuki 2012-05-17 0:07 ` KAMEZAWA Hiroyuki 2012-05-17 3:09 ` Glauber Costa 2012-05-17 3:09 ` Glauber Costa 2012-05-17 3:09 ` Glauber Costa
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20120514181250.GD2366@google.com \ --to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \ --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \ --cc=glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org \ --cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \ --cc=kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org \ --cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \ --cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \ --cc=mhocko-AlSwsSmVLrQ@public.gmane.org \ --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.