From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1759593AbcIMXfF (ORCPT <rfc822;w@1wt.eu>);
        Tue, 13 Sep 2016 19:35:05 -0400
Received: from mga07.intel.com ([134.134.136.100]:44024 "EHLO mga07.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753442AbcIMXfE (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 13 Sep 2016 19:35:04 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.30,331,1470726000"; 
   d="scan'208";a="8463834"
Date: Tue, 13 Sep 2016 16:35:02 -0700
From: "Luck, Tony" <tony.luck@intel.com>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        "H. Peter Anvin" <h.peter.anvin@intel.com>,
        Ingo Molnar <mingo@elte.hu>, Peter Zijlstra <peterz@infradead.org>,
        Tejun Heo <tj@kernel.org>, Borislav Petkov <bp@suse.de>,
        Stephane Eranian <eranian@google.com>,
        Marcelo Tosatti <mtosatti@redhat.com>,
        David Carrillo-Cisneros <davidcc@google.com>, Shaohua Li <shli@fb.com>,
        Ravi V Shankar <ravi.v.shankar@intel.com>,
        Vikas Shivappa <vikas.shivappa@linux.intel.com>,
        Sai Prakhya <sai.praneeth.prakhya@intel.com>,
        linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>
Subject: Re: [PATCH v2 26/33] Task fork and exit for rdtgroup
Message-ID: <20160913233502.GA4444@intel.com>
References: <1473328647-33116-1-git-send-email-fenghua.yu@intel.com>
 <1473328647-33116-27-git-send-email-fenghua.yu@intel.com>
 <57D88800.1090302@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <57D88800.1090302@intel.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Sep 13, 2016 at 04:13:04PM -0700, Dave Hansen wrote:
> Yikes, is this a new global lock and possible atomic_inc() on a shared
> variable in the fork() path?  Has there been any performance or
> scalability testing done on this code?
> 
> That mutex could be a disaster for fork() once the filesystem is
> mounted.  Even if it goes away, if you have a large number of processes
> in an rdtgroup and they are forking a lot, you're bound to see the
> rdtgrp->refcount get bounced around a lot.

The mutex is (almost certainly) going away.  The atomic_inc()
is likely staying (but only applies to tasks that are in 
resource groups other than the default one.  But on a system
where we partition the cache between containers/VMs, that may
essentially be all processes.

We only really use the refcount to decide whether the group
can be removed ... since that is the rare operation, perhaps
we could put all the work there and have it count them with:

	n = 0;
	rcu_read_lock();
	for_each_process(p)
		if (p->rdtgroup == this_rdtgroup)
			n++;
	rcu_read_unlock();
	if (n != 0)
		return -EBUSY;

then we might get the fork hook down to just:

void rdtgroup_fork(struct task_struct *child)
{
	child->rdtgroup = current->rdtgroup;
}

which looks a lot less scary :-)

-Tony