[PATCH i-g-t] lib/igt_core: Free log_buffer entries on exit
Kamil Konieczny
kamil.konieczny at linux.intel.com
Thu Nov 7 17:31:45 UTC 2024
Hi Lucas,
On 2024-11-06 at 23:02:08 -0800, Lucas De Marchi wrote:
> Make sure the buffer entries are released, otherwise valgrind output
> becomes each used entry as still reachable on exit (up to 256).
>
> Test:
> # valgrind --leak-check=full ./build/tests/xe_live_ktest --r xe_mocs
>
> Before:
> ==16082== LEAK SUMMARY:
> ==16082== definitely lost: 0 bytes in 0 blocks
> ==16082== indirectly lost: 0 bytes in 0 blocks
> ==16082== possibly lost: 0 bytes in 0 blocks
> ==16082== still reachable: 33,473 bytes in 59 blocks
>
> After:
> ==15992== LEAK SUMMARY:
> ==15992== definitely lost: 0 bytes in 0 blocks
> ==15992== indirectly lost: 0 bytes in 0 blocks
> ==15992== possibly lost: 0 bytes in 0 blocks
> ==15992== still reachable: 31,083 bytes in 42 blocks
Thank you for a patch, there are still some blocks left,
maybe because that buffer is a ring? e.g. it could have
end < start
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
> lib/igt_core.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 407f7b551..879d1ecd8 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -469,11 +469,21 @@ static void _igt_log_buffer_append(char *line)
> pthread_mutex_unlock(&log_buffer_mutex);
> }
>
> +static void _igt_log_buffer_reset_locked(void)
> +{
> + for (; log_buffer.start != log_buffer.end; log_buffer.start++) {
> + free(log_buffer.entries[log_buffer.start]);
> + log_buffer.entries[log_buffer.start] = NULL;
> + }
Here you should use the same loop as in _dump, so
i = log_buffer.start;
do {
// do ops here
i++;
} while (i != log_buffer.start && i != log_buffer.end);
Or even simpler, init with NULLs and then just free all?
For example, if start=2, end=1, there is still a valid pointer
at end==1
Or do one more ops for case 'end < start'
Regards,
Kamil
> +
> + log_buffer.start = log_buffer.end = 0;
> +}
> +
> static void _igt_log_buffer_reset(void)
> {
> pthread_mutex_lock(&log_buffer_mutex);
>
> - log_buffer.start = log_buffer.end = 0;
> + _igt_log_buffer_reset_locked();
>
> pthread_mutex_unlock(&log_buffer_mutex);
> }
> @@ -624,8 +634,7 @@ static void _igt_log_buffer_dump(void)
> i++;
> } while (i != log_buffer.start && i != log_buffer.end);
>
> - /* reset the buffer */
> - log_buffer.start = log_buffer.end = 0;
> + _igt_log_buffer_reset_locked();
>
> _log_line_fprintf(stderr, "**** END ****\n");
> pthread_mutex_unlock(&log_buffer_mutex);
> --
> 2.47.0
>
More information about the igt-dev
mailing list