Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Jul 2023 13:33:25 +0300 (EEST)
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>,
        Fenghua Yu <fenghua.yu@intel.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 09/19] selftests/resctrl: Convert span to size_t
In-Reply-To: <92b3ef73-6347-b52a-69ed-c1b489b11d55@intel.com>
Message-ID: <4969cf83-f070-f29-5489-1cc6a248c6a7@linux.intel.com>
References: <20230713131932.133258-1-ilpo.jarvinen@linux.intel.com> <20230713131932.133258-10-ilpo.jarvinen@linux.intel.com> <92b3ef73-6347-b52a-69ed-c1b489b11d55@intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; BOUNDARY="8323329-1953778464-1689329730=:1695"
Content-ID: <bd9e6e9-6dd-9a8d-602c-837010c9891@linux.intel.com>
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-1953778464-1689329730=:1695
Content-Type: text/plain; CHARSET=ISO-8859-15
Content-Transfer-Encoding: 8BIT
Content-ID: <92a9314-13ff-8d2b-8649-b666807b483d@linux.intel.com>

On Thu, 13 Jul 2023, Reinette Chatre wrote:

> Hi Ilpo,
> 
> On 7/13/2023 6:19 AM, Ilpo J?rvinen wrote:
> 
> ...
> 
> > @@ -188,10 +188,10 @@ fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
> >  	return 0;
> >  }
> >  
> > -int run_fill_buf(unsigned long span, int malloc_and_init_memory,
> > -		 int memflush, int op, char *resctrl_val)
> > +int run_fill_buf(size_t span, int malloc_and_init_memory, int memflush, int op,
> > +		 char *resctrl_val)
> >  {
> > -	unsigned long long cache_size = span;
> > +	size_t cache_size = span;
> >  	int ret;
> >  
> >  	ret = fill_cache(cache_size, malloc_and_init_memory, memflush, op,
> 
> Any idea what the purpose being run_fill_buf() is? From what I can tell it is
> an unnecessary level of indirection.

You already mentioned it, fill_cache() could be included into 
run_fill_buf() but it's not part of this series.

> > diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> > index f622245adafe..8be5b745226d 100644
> > --- a/tools/testing/selftests/resctrl/resctrlfs.c
> > +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> > @@ -298,7 +298,7 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no)
> >  void run_benchmark(int signum, siginfo_t *info, void *ucontext)
> >  {
> >  	int operation, ret, malloc_and_init_memory, memflush;
> > -	unsigned long span, buffer_span;
> > +	size_t span, buffer_span;
> >  	char **benchmark_cmd;
> >  	char resctrl_val[64];
> >  	FILE *fp;
> 
> Do we now need a cast in the initialization of span?

I don't see any warning w/o cast, unsigned long -> size_t seems pretty 
safe anyway. For internally provided values, overflow does not seem 
possible even if the type sizes would disagree.

There's no existing error checking for the command in the case where 
"fill_buf" is used with -b (an orthogonal issue).

-- 
 i.
--8323329-1953778464-1689329730=:1695--