Re: [RFC 02/18] cgroup_pids: track maximum pids
[Posted July 13, 2016 by jake]
| From: |
| Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A-AT-public.gmane.org> |
| To: |
| Topi Miettinen <toiwoton-Re5JQEeQqe8AvxtiuMwx3w-AT-public.gmane.org> |
| Subject: |
| Re: [RFC 02/18] cgroup_pids: track maximum pids |
| Date: |
| Mon, 13 Jun 2016 17:12:27 -0400 |
| Message-ID: |
| <20160613211227.GG31708@htj.duckdns.org> |
| Cc: |
| linux-kernel-u79uwXL29TY76Z2rM5mHXA-AT-public.gmane.org, Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA-AT-public.gmane.org>, Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w-AT-public.gmane.org>, "open list:CONTROL GROUP (CGROUP)" <cgroups-u79uwXL29TY76Z2rM5mHXA-AT-public.gmane.org> |
| Archive‑link: | |
Article |
Hello,
On Mon, Jun 13, 2016 at 10:44:09PM +0300, Topi Miettinen wrote:
> Track maximum pids in the cgroup, present it in cgroup pids.current_max.
"max" is often used for maximum limits in cgroup. I think "watermark"
or "high_watermark" would be a lot clearer.
> @@ -236,6 +246,14 @@ static void pids_free(struct task_struct *task)
> pids_uncharge(pids, 1);
> }
>
> +static void pids_fork(struct task_struct *task)
> +{
> + struct pids_cgroup *pids = css_pids(task_css(task, pids_cgrp_id));
> +
> + if (atomic64_read(&pids->cur_max) < atomic64_read(&pids->counter))
> + atomic64_set(&pids->cur_max, atomic64_read(&pids->counter));
> +}
Wouldn't it make more sense to track high watermark from the charge
functions instead? I don't get why this requires a separate fork
callback. Also, racing atomic64_set's are racy. The counter can end
up with a lower number than it should be.
> @@ -300,6 +326,11 @@ static struct cftype pids_files[] = {
> .read_s64 = pids_current_read,
> .flags = CFTYPE_NOT_ON_ROOT,
> },
> + {
> + .name = "current_max",
Please make this "high_watermark" field in pids.stats file.
Thanks.
--
tejun
