* [PATCH 1/2] ring_buffer: fix typing mistake
2009-02-09 15:17 [PATCH 0/2] git pull request for tip/tracing/urgent Steven Rostedt
@ 2009-02-09 15:17 ` Steven Rostedt
2009-02-09 15:17 ` [PATCH 2/2] ring_buffer: fix ring_buffer_read_page() Steven Rostedt
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2009-02-09 15:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Lai Jiangshan, Steven Rostedt
[-- Attachment #1: 0001-ring_buffer-fix-typing-mistake.patch --]
[-- Type: text/plain, Size: 2169 bytes --]
From: Lai Jiangshan <laijs@cn.fujitsu.com>
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.
[
This bug was a typo. The commit->reader_page is a page pointer
and not an index into the page. The line should have been
commit->reader_page->read += read. The other changes
by Lai are nice clean ups to the code. - SDR
]
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
kernel/trace/ring_buffer.c | 9 ++++-----
1 files 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);
--
1.5.6.5
--
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] ring_buffer: fix ring_buffer_read_page()
2009-02-09 15:17 [PATCH 0/2] git pull request for tip/tracing/urgent Steven Rostedt
2009-02-09 15:17 ` [PATCH 1/2] ring_buffer: fix typing mistake Steven Rostedt
@ 2009-02-09 15:17 ` Steven Rostedt
2009-02-09 15:22 ` [PATCH 0/2] git pull request for tip/tracing/urgent Steven Rostedt
2009-02-10 13:04 ` Ingo Molnar
3 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2009-02-09 15:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Lai Jiangshan, Steven Rostedt
[-- Attachment #1: 0002-ring_buffer-fix-ring_buffer_read_page.patch --]
[-- Type: text/plain, Size: 4034 bytes --]
From: Lai Jiangshan <laijs@cn.fujitsu.com>
Impact: change API and init bpage when copy
ring_buffer_read_page()/rb_remove_entries() may be called for
a partially consumed page.
Add a parameter for rb_remove_entries() and make it update
cpu_buffer->entries correctly for partially consumed pages.
ring_buffer_read_page() now returns the offset to the next event.
Init the bpage's time_stamp when return value is 0.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
kernel/trace/ring_buffer.c | 33 ++++++++++++++++++++-------------
1 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index e97acae..eaec7e5 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2303,13 +2303,14 @@ int ring_buffer_swap_cpu(struct ring_buffer *buffer_a,
EXPORT_SYMBOL_GPL(ring_buffer_swap_cpu);
static void rb_remove_entries(struct ring_buffer_per_cpu *cpu_buffer,
- struct buffer_data_page *bpage)
+ struct buffer_data_page *bpage,
+ unsigned int offset)
{
struct ring_buffer_event *event;
unsigned long head;
__raw_spin_lock(&cpu_buffer->lock);
- for (head = 0; head < local_read(&bpage->commit);
+ for (head = offset; head < local_read(&bpage->commit);
head += rb_event_length(event)) {
event = __rb_data_page_index(bpage, head);
@@ -2381,8 +2382,8 @@ void ring_buffer_free_read_page(struct ring_buffer *buffer, void *data)
* if (!rpage)
* return error;
* ret = ring_buffer_read_page(buffer, &rpage, cpu, 0);
- * if (ret)
- * process_page(rpage);
+ * if (ret >= 0)
+ * process_page(rpage, ret);
*
* When @full is set, the function will not return true unless
* the writer is off the reader page.
@@ -2393,8 +2394,8 @@ void ring_buffer_free_read_page(struct ring_buffer *buffer, void *data)
* responsible for that.
*
* Returns:
- * 1 if data has been transferred
- * 0 if no data has been transferred.
+ * >=0 if data has been transferred, returns the offset of consumed data.
+ * <0 if no data has been transferred.
*/
int ring_buffer_read_page(struct ring_buffer *buffer,
void **data_page, int cpu, int full)
@@ -2403,7 +2404,8 @@ int ring_buffer_read_page(struct ring_buffer *buffer,
struct ring_buffer_event *event;
struct buffer_data_page *bpage;
unsigned long flags;
- int ret = 0;
+ unsigned int read;
+ int ret = -1;
if (!data_page)
return 0;
@@ -2425,24 +2427,29 @@ int ring_buffer_read_page(struct ring_buffer *buffer,
/* check for data */
if (!local_read(&cpu_buffer->reader_page->page->commit))
goto out;
+
+ read = cpu_buffer->reader_page->read;
/*
* If the writer is already off of the read page, then simply
* switch the read page with the given page. Otherwise
* we need to copy the data from the reader to the writer.
*/
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);
+ struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
if (full)
goto out;
/* The writer is still on the reader page, we must copy */
- memcpy(bpage->data,
- cpu_buffer->reader_page->page->data + read,
- commit - read);
+ memcpy(bpage->data + read, rpage->data + read, commit - read);
/* consume what was read */
cpu_buffer->reader_page->read = commit;
+
+ /* update bpage */
+ local_set(&bpage->commit, commit);
+ if (!read)
+ bpage->time_stamp = rpage->time_stamp;
} else {
/* swap the pages */
rb_init_page(bpage);
@@ -2451,10 +2458,10 @@ int ring_buffer_read_page(struct ring_buffer *buffer,
cpu_buffer->reader_page->read = 0;
*data_page = bpage;
}
- ret = 1;
+ ret = read;
/* update the entry counter */
- rb_remove_entries(cpu_buffer, bpage);
+ rb_remove_entries(cpu_buffer, bpage, read);
out:
spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
--
1.5.6.5
--
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] git pull request for tip/tracing/urgent
2009-02-09 15:17 [PATCH 0/2] git pull request for tip/tracing/urgent Steven Rostedt
` (2 preceding siblings ...)
2009-02-09 15:22 ` [PATCH 0/2] git pull request for tip/tracing/urgent Steven Rostedt
@ 2009-02-10 13:04 ` Ingo Molnar
2009-02-10 14:11 ` Steven Rostedt
3 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2009-02-10 13:04 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, Andrew Morton, Lai Jiangshan
* Steven Rostedt <rostedt@goodmis.org> wrote:
> Ingo,
>
> Here's the changes by Lai. The bug scared me enough to investigate
> it further, to see if we had other bug reports that this could
> have fixed. But I found out that ring_buffer_read_page currently
> does not have any users. The only user I have is for my splice work,
> which has not been included yet.
>
> I guess Lai is adding one. These are true bug fixes to code that
> is in the kernel, even if that code is currently "dead code". But I think
> they should still be included in 29, since this will reflect on ftrace
> when we have developers starting to use it (such as Lai).
>
> The following patches are in:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
>
> branch: tip/tracing/urgent/devel
hm, the branch does not exist anymore:
fatal: Couldn't find remote ref tip/tracing/urgent/devel
fatal: Couldn't find remote ref tracing/urgent/devel
fatal: Couldn't find remote ref tracing/urgent
In any case, please include this in the devel path not the urgent path.
Urgent patches are for real regressions that occur with the upstream
kernel and affect users. New uses of tracing code should be done on
top of the tracing tree.
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread