From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423708AbcFMVMc (ORCPT ); Mon, 13 Jun 2016 17:12:32 -0400 Received: from mail-qk0-f193.google.com ([209.85.220.193]:35264 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161222AbcFMVMa (ORCPT ); Mon, 13 Jun 2016 17:12:30 -0400 Date: Mon, 13 Jun 2016 17:12:27 -0400 From: Tejun Heo To: Topi Miettinen Cc: linux-kernel@vger.kernel.org, Li Zefan , Johannes Weiner , "open list:CONTROL GROUP (CGROUP)" Subject: Re: [RFC 02/18] cgroup_pids: track maximum pids Message-ID: <20160613211227.GG31708@htj.duckdns.org> References: <1465847065-3577-1-git-send-email-toiwoton@gmail.com> <1465847065-3577-3-git-send-email-toiwoton@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1465847065-3577-3-git-send-email-toiwoton@gmail.com> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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