From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752829AbcFUROW (ORCPT ); Tue, 21 Jun 2016 13:14:22 -0400 Received: from mail-yw0-f195.google.com ([209.85.161.195]:36330 "EHLO mail-yw0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752173AbcFUROQ (ORCPT ); Tue, 21 Jun 2016 13:14:16 -0400 Date: Tue, 21 Jun 2016 13:12:12 -0400 From: Tejun Heo To: Kenny Yu Cc: lizefan@huawei.com, hannes@cmpxchg.org, cyphar@cyphar.com, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit Message-ID: <20160621171212.GL3262@mtj.duckdns.org> References: <20160621163402.GI3262@mtj.duckdns.org> <1466528198-3416939-1-git-send-email-kennyyu@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1466528198-3416939-1-git-send-email-kennyyu@fb.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, Just a couple nits. On Tue, Jun 21, 2016 at 09:56:38AM -0700, Kenny Yu wrote: > Summary: No need for "Summary:" tag. > This patch adds more visibility into the pids controller when the controller > rejects a fork request. Whenever fork fails because the limit on the number of > pids in the cgroup is reached, the controller will log this and also notify the > newly added cgroups events file. The `max` key in the events file represents > the number of times fork failed because of the pids controller. > > This change also adds an atomic boolean to prevent logging too much (e.g. a fork > bomb). The message is logged once per cgroup until the next time the pids limit > changes. The above paragraph isn't uptodate anymore. > @@ -213,10 +220,23 @@ static int pids_can_fork(struct task_struct *task) > { > struct cgroup_subsys_state *css; > struct pids_cgroup *pids; > + int err; > + int events_limit; > > css = task_css_check(current, pids_cgrp_id, true); > pids = css_pids(css); > - return pids_try_charge(pids, 1); > + err = pids_try_charge(pids, 1); > + if (err) { > + events_limit = atomic64_inc_return(&pids->events_limit); > + cgroup_file_notify(&pids->events_file); > + /* Only log the first time events_limit is incremented. */ > + if (events_limit == 1) { > + pr_info("cgroup: fork rejected by pids controller in "); > + pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id)); > + pr_cont("\n"); > + } > + } > + return err; > } It'd be better to use atomic64_inc_and_test() instead. if (err) { if (atomic64_inc_and_test()) { pr_xxx...; } cgroup_file_notify(&pids->events_file); } Thanks. -- tejun