Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Fri, 3 Nov 2023 10:53:39 +0200 (EET)
From:   =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
To:     Reinette Chatre <reinette.chatre@intel.com>
cc:     linux-kselftest@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
        Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>,
        =?ISO-8859-15?Q?Maciej_Wiecz=F3r-Retman?= 
        <maciej.wieczor-retman@intel.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 05/24] selftests/resctrl: Create cache_size() helper
In-Reply-To: <3849d986-caf4-41ad-85c7-11f11f986e1c@intel.com>
Message-ID: <12d53aae-177f-ff1f-306a-607eeae17ce3@linux.intel.com>
References: <20231024092634.7122-1-ilpo.jarvinen@linux.intel.com> <20231024092634.7122-6-ilpo.jarvinen@linux.intel.com> <3849d986-caf4-41ad-85c7-11f11f986e1c@intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-1015571647-1699001628=:1725"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-1015571647-1699001628=:1725
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT

On Thu, 2 Nov 2023, Reinette Chatre wrote:

> Hi Ilpo,
> 
> On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:
> > CAT and CMT tests calculate the span size from the n-bits cache
> > allocation on their own.
> > 
> > Add cache_size() helper which calculates size of the cache portion for
> > the given number of bits and use it to replace the existing span
> > calculations. This also prepares for the new CAT test that will need to
> > determine the size of the cache portion also during results processing.
> > 
> > cache_size local variables were renamed out of the way to
> > cache_total_size.
> 
> Please do stick to imperative mood ... "Rename cache_size local
> variables ..."
> 
> 
> ...
> 
> > diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> > index 2f3f0ee439d8..da06b2d492f9 100644
> > --- a/tools/testing/selftests/resctrl/resctrl.h
> > +++ b/tools/testing/selftests/resctrl/resctrl.h
> > @@ -117,4 +117,18 @@ int show_cache_info(unsigned long sum_llc_val, int no_of_bits,
> >  		    unsigned long max_diff_percent, unsigned long num_of_runs,
> >  		    bool platform, bool cmt);
> >  
> > +/*
> > + * cache_size - Calculate the size of a cache portion
> > + * @cache_size:	Cache size in bytes
> > + * @mask:	Cache portion mask
> > + * @cache_mask:	Full bitmask for the cache
> > + *
> > + * Return: The size of the cache portion in bytes.
> > + */
> > +static inline int cache_size(unsigned long cache_size, unsigned long mask,
> > +			     unsigned long cache_mask)
> > +{
> > +	return cache_size * count_bits(mask) / count_bits(cache_mask);
> > +}
> > +
> >  #endif /* RESCTRL_H */
> 
> 
> The get_cache_size() and cache_size() naming appears similar enough to me
> to cause confusion. Considering the "portion" term above, what do you think
> of "cache_portion_size()" or even "cache_portion_bytes()"?

Yes, I'm more than happy to rename them. This naming was what you 
suggested earlier. (I used cache_alloc_size() or something like that
initially and you were against using "alloc" in the name.)

-- 
 i.

--8323329-1015571647-1699001628=:1725--