Re: [PATCH v2 1/1] tracing: fix bug in rb_per_cpu_empty() that might cause deadloop.

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 21 Jul 2021 03:50:16 +0000
Haoran Luo <www@aegistudio.net> wrote:

> +	/*
> +	 * If writers are committing on the head page, we just need
> +	 * to care about there're committed data, and the reader will
> +	 * swap reader page with head page when it is to read data.
> +	 */
> +	if (commit == head)
> +		return rb_page_commit(commit) == 0;
> +
> +	/*
> +	 * Writers are committing on page other than reader page and
> +	 * head page, there should always be content to read.
> +	 */
> +	return false;
>  }
>  

I like the patch, but could you swap the above two operations. I find
that return conditional in the if condition rather ugly. That is, can
you instead have:


	/*
	 * If writers are committing on a page other than the reader
	 * page and head page, there should always be content to read.
	 */
	if (commit != head)
		return false;

	/*
	 * Writers are committing on the head page, we just need
	 * to care about there're committed data, and the reader will
	 * swap reader page with head page when it is to read data.
	 */
	return rb_page_commit(commit) == 0;

-- Steve