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