From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757209AbbKRV1Y (ORCPT ); Wed, 18 Nov 2015 16:27:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36221 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757028AbbKRV1X (ORCPT ); Wed, 18 Nov 2015 16:27:23 -0500 Date: Wed, 18 Nov 2015 19:27:04 -0200 From: Marcelo Tosatti To: Fenghua Yu Cc: H Peter Anvin , Ingo Molnar , Thomas Gleixner , Peter Zijlstra , linux-kernel , x86 , Vikas Shivappa Subject: Re: [PATCH V15 11/11] x86,cgroup/intel_rdt : Add a cgroup interface to manage Intel cache allocation Message-ID: <20151118212704.GB21918@amt.cnet> References: <1443766185-61618-1-git-send-email-fenghua.yu@intel.com> <1443766185-61618-12-git-send-email-fenghua.yu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1443766185-61618-12-git-send-email-fenghua.yu@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 01, 2015 at 11:09:45PM -0700, Fenghua Yu wrote: > Add a new cgroup 'intel_rdt' to manage cache allocation. Each cgroup > directory is associated with a class of service id(closid). To map a > task with closid during scheduling, this patch removes the closid field > from task_struct and uses the already existing 'cgroups' field in > task_struct. > > The cgroup has a file 'l3_cbm' which represents the L3 cache capacity > bitmask(CBM). The CBM is global for the whole system currently. The > capacity bitmask needs to have only contiguous bits set and number of > bits that can be set is less than the max bits that can be set. The > tasks belonging to a cgroup get to fill in the L3 cache represented by > the capacity bitmask of the cgroup. For ex: if the max bits in the CBM > is 10 and the cache size is 10MB, each bit represents 1MB of cache > capacity. > > Root cgroup always has all the bits set in the l3_cbm. User can create > more cgroups with mkdir syscall. By default the child cgroups inherit > the capacity bitmask(CBM) from parent. User can change the CBM specified > in hex for each cgroup. Each unique bitmask is associated with a class > of service ID and an -ENOSPC is returned once we run out of > closids. > > Signed-off-by: Vikas Shivappa > Signed-off-by: Fenghua Yu + clos_cbm_table_read(ir->closid, &ccbm); + if (cbmvalue == ccbm) + goto out; + + err = cbm_validate_rdt_cgroup(ir, cbmvalue); + if (err) + goto out; + + /* + * Try to get a reference for a different CLOSid and release the + * reference to the current CLOSid. + * Need to put down the reference here and get it back in case we + * run out of closids. Otherwise we run into a problem when + * we could be using the last closid that could have been available. + */ + closid_put(ir->closid); + if (cbm_search(cbmvalue, &closid)) { Can't you move closid_put here? + ir->closid = closid; + closid_get(closid); + } else { + closid = ir->closid; Variable unused. + err = closid_alloc(&ir->closid); + if (err) { + closid_get(ir->closid); + goto out; + } This makes you cycle closid when changing the cbm, not necessary. (not very important, but closid_put is nerving because it can possibly set l3_cbm to zero).