[V1,09/13] selftests/resctrl: Modularize fill_buf for new CAT test case
diff mbox series

Message ID 43b368952bb006ee973311d9c9ae0eb53d8e7f60.1583657204.git.sai.praneeth.prakhya@intel.com
State New
Headers show
Series
  • Miscellaneous fixes for resctrl selftests
Related show

Commit Message

Prakhya, Sai Praneeth March 7, 2020, 3:40 a.m. UTC
Currently fill_buf (in-built benchmark) runs as a separate process and it
runs indefinitely looping around given buffer either reading it or writing
to it. But, some future test cases might want to start and stop looping
around the buffer as they see fit. So, modularize fill_buf to support this
use case.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
---
 tools/testing/selftests/resctrl/fill_buf.c | 66 ++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 22 deletions(-)

Comments

Reinette Chatre March 10, 2020, 9:59 p.m. UTC | #1
Hi Sai,

On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote:
> Currently fill_buf (in-built benchmark) runs as a separate process and it
> runs indefinitely looping around given buffer either reading it or writing
> to it. But, some future test cases might want to start and stop looping
> around the buffer as they see fit. So, modularize fill_buf to support this
> use case.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> ---
>  tools/testing/selftests/resctrl/fill_buf.c | 66 ++++++++++++++++++++----------
>  1 file changed, 44 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
> index 9ede7b63f059..204ae8870a32 100644
> --- a/tools/testing/selftests/resctrl/fill_buf.c
> +++ b/tools/testing/selftests/resctrl/fill_buf.c
> @@ -23,7 +23,7 @@
>  #define PAGE_SIZE		(4 * 1024)
>  #define MB			(1024 * 1024)
>  
> -static unsigned char *startptr;
> +static unsigned char *startptr, *endptr;
>  
>  static void sb(void)
>  {
> @@ -82,13 +82,13 @@ static void *malloc_and_init_memory(size_t s)
>  	return p;
>  }
>  
> -static int fill_one_span_read(unsigned char *start_ptr, unsigned char *end_ptr)
> +static int fill_one_span_read(void)
>  {
>  	unsigned char sum, *p;
>  
>  	sum = 0;
> -	p = start_ptr;
> -	while (p < end_ptr) {
> +	p = startptr;
> +	while (p < endptr) {
>  		sum += *p;
>  		p += (CL_SIZE / 2);
>  	}
> @@ -96,26 +96,24 @@ static int fill_one_span_read(unsigned char *start_ptr, unsigned char *end_ptr)
>  	return sum;
>  }
>  
> -static
> -void fill_one_span_write(unsigned char *start_ptr, unsigned char *end_ptr)
> +static void fill_one_span_write(void)
>  {
>  	unsigned char *p;
>  
> -	p = start_ptr;
> -	while (p < end_ptr) {
> +	p = startptr;
> +	while (p < endptr) {
>  		*p = '1';
>  		p += (CL_SIZE / 2);
>  	}
>  }
>  
> -static int fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr,
> -			   char *resctrl_val)
> +static int fill_cache_read(char *resctrl_val)
>  {
>  	int ret = 0;
>  	FILE *fp;
>  
>  	while (1) {
> -		ret = fill_one_span_read(start_ptr, end_ptr);
> +		ret = fill_one_span_read();
>  		if (!strcmp(resctrl_val, "cat"))
>  			break;
>  	}
> @@ -130,11 +128,10 @@ static int fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr,
>  	return 0;
>  }
>  
> -static int fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr,
> -			    char *resctrl_val)
> +static int fill_cache_write(char *resctrl_val)
>  {
>  	while (1) {
> -		fill_one_span_write(start_ptr, end_ptr);
> +		fill_one_span_write();
>  		if (!strcmp(resctrl_val, "cat"))
>  			break;
>  	}
> @@ -142,24 +139,25 @@ static int fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr,
>  	return 0;
>  }
>  
> -static int
> -fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
> -	   int op, char *resctrl_val)
> +static
> +int init_buffer(unsigned long long buf_size, int malloc_and_init, int memflush)
>  {
>  	unsigned char *start_ptr, *end_ptr;
>  	unsigned long long i;
> -	int ret;
>  
>  	if (malloc_and_init)
>  		start_ptr = malloc_and_init_memory(buf_size);
>  	else
>  		start_ptr = malloc(buf_size);
>  
> -	if (!start_ptr)
> +	if (!start_ptr) {
> +		printf("Failed to allocate memory to buffer\n");
>  		return -1;
> +	}
>  
> -	startptr = start_ptr;
>  	end_ptr = start_ptr + buf_size;
> +	endptr = end_ptr;
> +	startptr = start_ptr;
>  
>  	/*
>  	 * It's better to touch the memory once to avoid any compiler
> @@ -176,16 +174,40 @@ fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
>  	if (memflush)
>  		mem_flush(start_ptr, buf_size);
>  
> +	return 0;
> +}
> +
> +static int use_buffer_forever(int op, char *resctrl_val)
> +{
> +	int ret;
> +
>  	if (op == 0)
> -		ret = fill_cache_read(start_ptr, end_ptr, resctrl_val);
> +		ret = fill_cache_read(resctrl_val);
>  	else
> -		ret = fill_cache_write(start_ptr, end_ptr, resctrl_val);
> +		ret = fill_cache_write(resctrl_val);
>  
>  	if (ret) {
>  		printf("\n Errror in fill cache read/write...\n");
>  		return -1;
>  	}
>  
> +	return 0;
> +}
> +
> +static int
> +fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
> +	   int op, char *resctrl_val)
> +{
> +	int ret;
> +
> +	ret = init_buffer(buf_size, malloc_and_init, memflush);
> +	if (ret)
> +		return ret;
> +
> +	ret = use_buffer_forever(op, resctrl_val);
> +	if (ret)
> +		return ret;

Should buffer be freed on this error path?

I think the asymmetrical nature of the memory allocation and release
creates traps like this.

It may be less error prone to have the pointer returned by init_buffer
and the acted on and released within fill_cache(), passed to
"use_buffer_forever()" as a parameter.  The buffer size is known here,
there is no need to keep an "end pointer" around.

> +
>  	free(startptr);
>  
>  	return 0;
> 

Reinette
Prakhya, Sai Praneeth March 11, 2020, 1:04 a.m. UTC | #2
Hi Reinette,

On Tue, 2020-03-10 at 14:59 -0700, Reinette Chatre wrote:
> Hi Sai,
> 
> On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote:
> > Currently fill_buf (in-built benchmark) runs as a separate process and it
> > runs indefinitely looping around given buffer either reading it or writing
> > to it. But, some future test cases might want to start and stop looping
> > around the buffer as they see fit. So, modularize fill_buf to support this
> > use case.
> > 
> > Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > ---
> >  tools/testing/selftests/resctrl/fill_buf.c | 66 ++++++++++++++++++++-----
> > -----
> >  1 file changed, 44 insertions(+), 22 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/resctrl/fill_buf.c
> > b/tools/testing/selftests/resctrl/fill_buf.c
> > index 9ede7b63f059..204ae8870a32 100644
> > --- a/tools/testing/selftests/resctrl/fill_buf.c
> > +++ b/tools/testing/selftests/resctrl/fill_buf.c
> > @@ -23,7 +23,7 @@
> >  #define PAGE_SIZE		(4 * 1024)
> >  #define MB			(1024 * 1024)
> >  
> > -static unsigned char *startptr;
> > +static unsigned char *startptr, *endptr;

[Snipped.. assuming code over here might not be needed for discussion]

> > +static int use_buffer_forever(int op, char *resctrl_val)
> > +{
> > +	int ret;
> > +
> >  	if (op == 0)
> > -		ret = fill_cache_read(start_ptr, end_ptr, resctrl_val);
> > +		ret = fill_cache_read(resctrl_val);
> >  	else
> > -		ret = fill_cache_write(start_ptr, end_ptr, resctrl_val);
> > +		ret = fill_cache_write(resctrl_val);
> >  
> >  	if (ret) {
> >  		printf("\n Errror in fill cache read/write...\n");
> >  		return -1;
> >  	}
> >  
> > +	return 0;
> > +}
> > +
> > +static int
> > +fill_cache(unsigned long long buf_size, int malloc_and_init, int
> > memflush,
> > +	   int op, char *resctrl_val)
> > +{
> > +	int ret;
> > +
> > +	ret = init_buffer(buf_size, malloc_and_init, memflush);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = use_buffer_forever(op, resctrl_val);
> > +	if (ret)
> > +		return ret;
> 
> Should buffer be freed on this error path?

Yes, that's right.. my bad. Will fix it. But the right fix is,
use_buffer_forever() should not return at all. It's meant to loop around the
buffer _forever_.

> I think the asymmetrical nature of the memory allocation and release
> creates traps like this.
> 
> It may be less error prone to have the pointer returned by init_buffer
> and the acted on and released within fill_cache(), passed to
> "use_buffer_forever()" as a parameter.  The buffer size is known here,
> there is no need to keep an "end pointer" around.

The main reason for having "startptr" as a global variable is to free memory
when fill_buf is killed. fill_buf runs as a separate process (for test cases
like MBM, MBA and CQM) and when user issues Ctrl_c or when the test kills
benchmark_pid (i.e. fill_buf), the buffer is freed (please see
ctrl_handler()).

So, I thought, as "startptr" is anyways global, why pass it around as an
argument? While making this change I thought it's natural to make "endptr"
global as well because the function didn't really look good to just take
endptr as an argument without startptr.

I do agree that asymmetrical nature of the memory allocation and release might
create traps, I will try to overcome this for CAT test case (other test cases
will not need it).

Regards,
Sai
Reinette Chatre March 11, 2020, 3:44 p.m. UTC | #3
Hi Sai,

On 3/10/2020 6:04 PM, Sai Praneeth Prakhya wrote:
> On Tue, 2020-03-10 at 14:59 -0700, Reinette Chatre wrote:
>> On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote:
>>> Currently fill_buf (in-built benchmark) runs as a separate process and it
>>> runs indefinitely looping around given buffer either reading it or writing
>>> to it. But, some future test cases might want to start and stop looping
>>> around the buffer as they see fit. So, modularize fill_buf to support this
>>> use case.
>>>
>>> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
>>> ---
>>>  tools/testing/selftests/resctrl/fill_buf.c | 66 ++++++++++++++++++++-----
>>> -----
>>>  1 file changed, 44 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/resctrl/fill_buf.c
>>> b/tools/testing/selftests/resctrl/fill_buf.c
>>> index 9ede7b63f059..204ae8870a32 100644
>>> --- a/tools/testing/selftests/resctrl/fill_buf.c
>>> +++ b/tools/testing/selftests/resctrl/fill_buf.c
>>> @@ -23,7 +23,7 @@
>>>  #define PAGE_SIZE		(4 * 1024)
>>>  #define MB			(1024 * 1024)
>>>  
>>> -static unsigned char *startptr;
>>> +static unsigned char *startptr, *endptr;
> 
> [Snipped.. assuming code over here might not be needed for discussion]
> 
>>> +static int use_buffer_forever(int op, char *resctrl_val)
>>> +{
>>> +	int ret;
>>> +
>>>  	if (op == 0)
>>> -		ret = fill_cache_read(start_ptr, end_ptr, resctrl_val);
>>> +		ret = fill_cache_read(resctrl_val);
>>>  	else
>>> -		ret = fill_cache_write(start_ptr, end_ptr, resctrl_val);
>>> +		ret = fill_cache_write(resctrl_val);
>>>  
>>>  	if (ret) {
>>>  		printf("\n Errror in fill cache read/write...\n");
>>>  		return -1;
>>>  	}
>>>  
>>> +	return 0;
>>> +}
>>> +
>>> +static int
>>> +fill_cache(unsigned long long buf_size, int malloc_and_init, int
>>> memflush,
>>> +	   int op, char *resctrl_val)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = init_buffer(buf_size, malloc_and_init, memflush);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = use_buffer_forever(op, resctrl_val);
>>> +	if (ret)
>>> +		return ret;
>>
>> Should buffer be freed on this error path?
> 
> Yes, that's right.. my bad. Will fix it. But the right fix is,
> use_buffer_forever() should not return at all. It's meant to loop around the
> buffer _forever_.
> 
>> I think the asymmetrical nature of the memory allocation and release
>> creates traps like this.
>>
>> It may be less error prone to have the pointer returned by init_buffer
>> and the acted on and released within fill_cache(), passed to
>> "use_buffer_forever()" as a parameter.  The buffer size is known here,
>> there is no need to keep an "end pointer" around.
> 
> The main reason for having "startptr" as a global variable is to free memory
> when fill_buf is killed. fill_buf runs as a separate process (for test cases
> like MBM, MBA and CQM) and when user issues Ctrl_c or when the test kills
> benchmark_pid (i.e. fill_buf), the buffer is freed (please see
> ctrl_handler()).

I see. Got it, thanks.

> 
> So, I thought, as "startptr" is anyways global, why pass it around as an
> argument? While making this change I thought it's natural to make "endptr"
> global as well because the function didn't really look good to just take
> endptr as an argument without startptr.

Maintaining the end pointer is unusual. The start of the buffer and the
size are known properties that the end of the buffer can be computed
from. Not a problem, it just seems inconsistent that some of the buffer
functions operate on the start pointer and size while others operate on
the start pointer and end pointer.

> I do agree that asymmetrical nature of the memory allocation and release might
> create traps, I will try to overcome this for CAT test case (other test cases
> will not need it).

Thank you very much

Reinette
Prakhya, Sai Praneeth March 11, 2020, 5:45 p.m. UTC | #4
Hi Reinette,

On Wed, 2020-03-11 at 08:44 -0700, Reinette Chatre wrote:
> Hi Sai,
> 
> On 3/10/2020 6:04 PM, Sai Praneeth Prakhya wrote:
> > On Tue, 2020-03-10 at 14:59 -0700, Reinette Chatre wrote:
> > > On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote:
> > > > Currently fill_buf (in-built benchmark) runs as a separate process a
> > > > 
> > > 

[SNIP]

> > > Should buffer be freed on this error path?
> > 
> > Yes, that's right.. my bad. Will fix it. But the right fix is,
> > use_buffer_forever() should not return at all. It's meant to loop around
> > the
> > buffer _forever_.
> > 
> > > I think the asymmetrical nature of the memory allocation and release
> > > creates traps like this.
> > > 
> > > It may be less error prone to have the pointer returned by init_buffer
> > > and the acted on and released within fill_cache(), passed to
> > > "use_buffer_forever()" as a parameter.  The buffer size is known here,
> > > there is no need to keep an "end pointer" around.
> > 
> > The main reason for having "startptr" as a global variable is to free
> > memory
> > when fill_buf is killed. fill_buf runs as a separate process (for test
> > cases
> > like MBM, MBA and CQM) and when user issues Ctrl_c or when the test kills
> > benchmark_pid (i.e. fill_buf), the buffer is freed (please see
> > ctrl_handler()).
> 
> I see. Got it, thanks.
> 
> > So, I thought, as "startptr" is anyways global, why pass it around as an
> > argument? While making this change I thought it's natural to make "endptr"
> > global as well because the function didn't really look good to just take
> > endptr as an argument without startptr.
> 
> Maintaining the end pointer is unusual. The start of the buffer and the
> size are known properties that the end of the buffer can be computed
> from. Not a problem, it just seems inconsistent that some of the buffer
> functions operate on the start pointer and size while others operate on
> the start pointer and end pointer.

Ok.. makes sense. I will try to make it consistent by using endptr all the
time. One advantage of using endptr is that we could just compute endptr once
and use it when needed by passing it as variable (will try to not make it
global variable).

Regards,
Sai
Reinette Chatre March 11, 2020, 6:10 p.m. UTC | #5
Hi Sai,

On 3/11/2020 10:45 AM, Sai Praneeth Prakhya wrote:
> On Wed, 2020-03-11 at 08:44 -0700, Reinette Chatre wrote:
>> On 3/10/2020 6:04 PM, Sai Praneeth Prakhya wrote:
>>> On Tue, 2020-03-10 at 14:59 -0700, Reinette Chatre wrote:
>>>> On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote:
>>>>> Currently fill_buf (in-built benchmark) runs as a separate process a
>>>>>
>>>>
> 
> [SNIP]
> 
>>>> Should buffer be freed on this error path?
>>>
>>> Yes, that's right.. my bad. Will fix it. But the right fix is,
>>> use_buffer_forever() should not return at all. It's meant to loop around
>>> the
>>> buffer _forever_.
>>>
>>>> I think the asymmetrical nature of the memory allocation and release
>>>> creates traps like this.
>>>>
>>>> It may be less error prone to have the pointer returned by init_buffer
>>>> and the acted on and released within fill_cache(), passed to
>>>> "use_buffer_forever()" as a parameter.  The buffer size is known here,
>>>> there is no need to keep an "end pointer" around.
>>>
>>> The main reason for having "startptr" as a global variable is to free
>>> memory
>>> when fill_buf is killed. fill_buf runs as a separate process (for test
>>> cases
>>> like MBM, MBA and CQM) and when user issues Ctrl_c or when the test kills
>>> benchmark_pid (i.e. fill_buf), the buffer is freed (please see
>>> ctrl_handler()).
>>
>> I see. Got it, thanks.
>>
>>> So, I thought, as "startptr" is anyways global, why pass it around as an
>>> argument? While making this change I thought it's natural to make "endptr"
>>> global as well because the function didn't really look good to just take
>>> endptr as an argument without startptr.
>>
>> Maintaining the end pointer is unusual. The start of the buffer and the
>> size are known properties that the end of the buffer can be computed
>> from. Not a problem, it just seems inconsistent that some of the buffer
>> functions operate on the start pointer and size while others operate on
>> the start pointer and end pointer.
> 
> Ok.. makes sense. I will try to make it consistent by using endptr all the
> time. One advantage of using endptr is that we could just compute endptr once
> and use it when needed by passing it as variable (will try to not make it
> global variable).

This may add unnecessary complexity because from what I can tell some of
those calls require buffer size and this would then require needing to
recompute the buffer size based on the start and end pointers. Do you
really need the end pointer? Can you not just use the start pointer and
buffer size?

Reinette
Prakhya, Sai Praneeth March 11, 2020, 6:14 p.m. UTC | #6
HI Reinette,

On Wed, 2020-03-11 at 11:10 -0700, Reinette Chatre wrote:
> Hi Sai,
> 
> On 3/11/2020 10:45 AM, Sai Praneeth Prakhya wrote:
> > On Wed, 2020-03-11 at 08:44 -0700, Reinette Chatre wrote:
> > > On 3/10/2020 6:04 PM, Sai Praneeth Prakhya wrote:
> > > > On Tue, 2020-03-10 at 14:59 -0700, Reinette Chatre wrote:
> > > > > On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote:
> > > > > > Currently fill_buf (in-built benchmark) runs as a separate process
> > > > > > a
> > > > > > 
> > 

[SNIP]

> > > Maintaining the end pointer is unusual. The start of the buffer and the
> > > size are known properties that the end of the buffer can be computed
> > > from. Not a problem, it just seems inconsistent that some of the buffer
> > > functions operate on the start pointer and size while others operate on
> > > the start pointer and end pointer.
> > 
> > Ok.. makes sense. I will try to make it consistent by using endptr all the
> > time. One advantage of using endptr is that we could just compute endptr
> > once
> > and use it when needed by passing it as variable (will try to not make it
> > global variable).
> 
> This may add unnecessary complexity because from what I can tell some of
> those calls require buffer size and this would then require needing to
> recompute the buffer size based on the start and end pointers. Do you
> really need the end pointer? Can you not just use the start pointer and
> buffer size?

Ok.. makes sense. Will use buffer size.

Regards,
Sai

Patch
diff mbox series

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
index 9ede7b63f059..204ae8870a32 100644
--- a/tools/testing/selftests/resctrl/fill_buf.c
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -23,7 +23,7 @@ 
 #define PAGE_SIZE		(4 * 1024)
 #define MB			(1024 * 1024)
 
-static unsigned char *startptr;
+static unsigned char *startptr, *endptr;
 
 static void sb(void)
 {
@@ -82,13 +82,13 @@  static void *malloc_and_init_memory(size_t s)
 	return p;
 }
 
-static int fill_one_span_read(unsigned char *start_ptr, unsigned char *end_ptr)
+static int fill_one_span_read(void)
 {
 	unsigned char sum, *p;
 
 	sum = 0;
-	p = start_ptr;
-	while (p < end_ptr) {
+	p = startptr;
+	while (p < endptr) {
 		sum += *p;
 		p += (CL_SIZE / 2);
 	}
@@ -96,26 +96,24 @@  static int fill_one_span_read(unsigned char *start_ptr, unsigned char *end_ptr)
 	return sum;
 }
 
-static
-void fill_one_span_write(unsigned char *start_ptr, unsigned char *end_ptr)
+static void fill_one_span_write(void)
 {
 	unsigned char *p;
 
-	p = start_ptr;
-	while (p < end_ptr) {
+	p = startptr;
+	while (p < endptr) {
 		*p = '1';
 		p += (CL_SIZE / 2);
 	}
 }
 
-static int fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr,
-			   char *resctrl_val)
+static int fill_cache_read(char *resctrl_val)
 {
 	int ret = 0;
 	FILE *fp;
 
 	while (1) {
-		ret = fill_one_span_read(start_ptr, end_ptr);
+		ret = fill_one_span_read();
 		if (!strcmp(resctrl_val, "cat"))
 			break;
 	}
@@ -130,11 +128,10 @@  static int fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr,
 	return 0;
 }
 
-static int fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr,
-			    char *resctrl_val)
+static int fill_cache_write(char *resctrl_val)
 {
 	while (1) {
-		fill_one_span_write(start_ptr, end_ptr);
+		fill_one_span_write();
 		if (!strcmp(resctrl_val, "cat"))
 			break;
 	}
@@ -142,24 +139,25 @@  static int fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr,
 	return 0;
 }
 
-static int
-fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
-	   int op, char *resctrl_val)
+static
+int init_buffer(unsigned long long buf_size, int malloc_and_init, int memflush)
 {
 	unsigned char *start_ptr, *end_ptr;
 	unsigned long long i;
-	int ret;
 
 	if (malloc_and_init)
 		start_ptr = malloc_and_init_memory(buf_size);
 	else
 		start_ptr = malloc(buf_size);
 
-	if (!start_ptr)
+	if (!start_ptr) {
+		printf("Failed to allocate memory to buffer\n");
 		return -1;
+	}
 
-	startptr = start_ptr;
 	end_ptr = start_ptr + buf_size;
+	endptr = end_ptr;
+	startptr = start_ptr;
 
 	/*
 	 * It's better to touch the memory once to avoid any compiler
@@ -176,16 +174,40 @@  fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
 	if (memflush)
 		mem_flush(start_ptr, buf_size);
 
+	return 0;
+}
+
+static int use_buffer_forever(int op, char *resctrl_val)
+{
+	int ret;
+
 	if (op == 0)
-		ret = fill_cache_read(start_ptr, end_ptr, resctrl_val);
+		ret = fill_cache_read(resctrl_val);
 	else
-		ret = fill_cache_write(start_ptr, end_ptr, resctrl_val);
+		ret = fill_cache_write(resctrl_val);
 
 	if (ret) {
 		printf("\n Errror in fill cache read/write...\n");
 		return -1;
 	}
 
+	return 0;
+}
+
+static int
+fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
+	   int op, char *resctrl_val)
+{
+	int ret;
+
+	ret = init_buffer(buf_size, malloc_and_init, memflush);
+	if (ret)
+		return ret;
+
+	ret = use_buffer_forever(op, resctrl_val);
+	if (ret)
+		return ret;
+
 	free(startptr);
 
 	return 0;