Messages in this thread | | | Date | Mon, 13 Jun 2016 17:12:27 -0400 | From | Tejun Heo <> | Subject | Re: [RFC 02/18] cgroup_pids: track maximum pids |
| |
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
| |