Re: [PATCH 1/2] ring_buffer: fix typing mistake

[Steven Rostedt]

On Mon, 9 Feb 2009, Lai Jiangshan wrote:

> 
> Impact: Fix bug
> 
> I found several very very curious line.
> It's so curious that it may be brought by typing mistake.
> 
> When (cpu_buffer->reader_page == cpu_buffer->commit_page):
> 
> 1) We haven't copied it for bpage is changed:
>    bpage = cpu_buffer->reader_page->page;
>    memcpy(bpage->data, cpu_buffer->reader_page->page->data + read ... )
> 2) We need update cpu_buffer->reader_page->read, but
>    "cpu_buffer->reader_page += read;" is not right.

Ouch! That is a bug.

Ingo, this is an urgent fix. Want me to pull it in and send it off to you?

-- Steve


> 
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> ---
>  ring_buffer.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index bd38c5c..e97acae 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -2377,7 +2377,7 @@ void ring_buffer_free_read_page(struct ring_buffer *buffer, void *data)
>   * to swap with a page in the ring buffer.
>   *
>   * for example:
> - *	rpage = ring_buffer_alloc_page(buffer);
> + *	rpage = ring_buffer_alloc_read_page(buffer);
>   *	if (!rpage)
>   *		return error;
>   *	ret = ring_buffer_read_page(buffer, &rpage, cpu, 0);
> @@ -2432,18 +2432,17 @@ int ring_buffer_read_page(struct ring_buffer *buffer,
>  	 */
>  	if (cpu_buffer->reader_page == cpu_buffer->commit_page) {
>  		unsigned int read = cpu_buffer->reader_page->read;
> +		unsigned int commit = rb_page_commit(cpu_buffer->reader_page);
>  
>  		if (full)
>  			goto out;
>  		/* The writer is still on the reader page, we must copy */
> -		bpage = cpu_buffer->reader_page->page;
>  		memcpy(bpage->data,
>  		       cpu_buffer->reader_page->page->data + read,
> -		       local_read(&bpage->commit) - read);
> +		       commit - read);
>  
>  		/* consume what was read */
> -		cpu_buffer->reader_page += read;
> -
> +		cpu_buffer->reader_page->read = commit;
>  	} else {
>  		/* swap the pages */
>  		rb_init_page(bpage);
> 
> 
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/