* [trace:ring-buffer/for-next 2/4] kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting
@ 2024-10-19 14:43 kernel test robot
2024-10-19 15:07 ` Steven Rostedt
0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2024-10-19 14:43 UTC (permalink / raw)
To: Petr Pavlu; +Cc: oe-kbuild-all, Steven Rostedt (Google)
tree: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace ring-buffer/for-next
head: 70c765743423ea51af71865aeb4530bcdf892300
commit: 1f1c2bc9d0753afb466ad6f0c2cd9dc08ed637fa [2/4] ring-buffer: Limit time with disabled interrupts in rb_check_pages()
config: i386-randconfig-141-20241019 (https://download.01.org/0day-ci/archive/20241019/202410192201.oCwXfxZf-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410192201.oCwXfxZf-lkp@intel.com/
smatch warnings:
kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting
vim +1540 kernel/trace/ring_buffer.c
1493
1494 /**
1495 * rb_check_pages - integrity check of buffer pages
1496 * @cpu_buffer: CPU buffer with pages to test
1497 *
1498 * As a safety measure we check to make sure the data pages have not
1499 * been corrupted.
1500 */
1501 static void rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
1502 {
1503 struct list_head *head, *tmp;
1504 unsigned long buffer_cnt;
1505 unsigned long flags;
1506 int nr_loops = 0;
1507
1508 /*
1509 * Walk the linked list underpinning the ring buffer and validate all
1510 * its next and prev links.
1511 *
1512 * The check acquires the reader_lock to avoid concurrent processing
1513 * with code that could be modifying the list. However, the lock cannot
1514 * be held for the entire duration of the walk, as this would make the
1515 * time when interrupts are disabled non-deterministic, dependent on the
1516 * ring buffer size. Therefore, the code releases and re-acquires the
1517 * lock after checking each page. The ring_buffer_per_cpu.cnt variable
1518 * is then used to detect if the list was modified while the lock was
1519 * not held, in which case the check needs to be restarted.
1520 *
1521 * The code attempts to perform the check at most three times before
1522 * giving up. This is acceptable because this is only a self-validation
1523 * to detect problems early on. In practice, the list modification
1524 * operations are fairly spaced, and so this check typically succeeds at
1525 * most on the second try.
1526 */
1527 again:
1528 if (++nr_loops > 3)
1529 return;
1530
1531 raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
1532 head = rb_list_head(cpu_buffer->pages);
1533 if (!rb_check_links(cpu_buffer, head))
1534 goto out_locked;
1535 buffer_cnt = cpu_buffer->cnt;
1536 tmp = head;
1537 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
1538 return;
1539
> 1540 while (true) {
1541 raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
1542
1543 if (buffer_cnt != cpu_buffer->cnt) {
1544 /* The list was updated, try again. */
1545 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
1546 goto again;
1547 }
1548
1549 tmp = rb_list_head(tmp->next);
1550 if (tmp == head)
1551 /* The iteration circled back, all is done. */
1552 goto out_locked;
1553
1554 if (!rb_check_links(cpu_buffer, tmp))
1555 goto out_locked;
1556
1557 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
1558 }
1559
1560 out_locked:
1561 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
1562 }
1563
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [trace:ring-buffer/for-next 2/4] kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting
2024-10-19 14:43 [trace:ring-buffer/for-next 2/4] kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting kernel test robot
@ 2024-10-19 15:07 ` Steven Rostedt
2024-10-19 15:14 ` Steven Rostedt
0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2024-10-19 15:07 UTC (permalink / raw)
To: kernel test robot; +Cc: Petr Pavlu, oe-kbuild-all
On Sat, 19 Oct 2024 22:43:44 +0800
kernel test robot <lkp@intel.com> wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace ring-buffer/for-next
> head: 70c765743423ea51af71865aeb4530bcdf892300
> commit: 1f1c2bc9d0753afb466ad6f0c2cd9dc08ed637fa [2/4] ring-buffer: Limit time with disabled interrupts in rb_check_pages()
> config: i386-randconfig-141-20241019 (https://download.01.org/0day-ci/archive/20241019/202410192201.oCwXfxZf-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202410192201.oCwXfxZf-lkp@intel.com/
>
> smatch warnings:
> kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting
>
> vim +1540 kernel/trace/ring_buffer.c
>
> 1493
> 1494 /**
> 1495 * rb_check_pages - integrity check of buffer pages
> 1496 * @cpu_buffer: CPU buffer with pages to test
> 1497 *
> 1498 * As a safety measure we check to make sure the data pages have not
> 1499 * been corrupted.
> 1500 */
> 1501 static void rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
> 1502 {
> 1503 struct list_head *head, *tmp;
> 1504 unsigned long buffer_cnt;
> 1505 unsigned long flags;
> 1506 int nr_loops = 0;
> 1507
> 1508 /*
> 1509 * Walk the linked list underpinning the ring buffer and validate all
> 1510 * its next and prev links.
> 1511 *
> 1512 * The check acquires the reader_lock to avoid concurrent processing
> 1513 * with code that could be modifying the list. However, the lock cannot
> 1514 * be held for the entire duration of the walk, as this would make the
> 1515 * time when interrupts are disabled non-deterministic, dependent on the
> 1516 * ring buffer size. Therefore, the code releases and re-acquires the
> 1517 * lock after checking each page. The ring_buffer_per_cpu.cnt variable
> 1518 * is then used to detect if the list was modified while the lock was
> 1519 * not held, in which case the check needs to be restarted.
> 1520 *
> 1521 * The code attempts to perform the check at most three times before
> 1522 * giving up. This is acceptable because this is only a self-validation
> 1523 * to detect problems early on. In practice, the list modification
> 1524 * operations are fairly spaced, and so this check typically succeeds at
> 1525 * most on the second try.
> 1526 */
> 1527 again:
> 1528 if (++nr_loops > 3)
> 1529 return;
> 1530
> 1531 raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
> 1532 head = rb_list_head(cpu_buffer->pages);
> 1533 if (!rb_check_links(cpu_buffer, head))
> 1534 goto out_locked;
> 1535 buffer_cnt = cpu_buffer->cnt;
> 1536 tmp = head;
> 1537 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> 1538 return;
Bah, an extra "return" was added due to me rebasing the patch incorrectly.
I'll fix and will need to rebase my for-next branch.
-- Steve
> 1539
> > 1540 while (true) {
> 1541 raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
> 1542
> 1543 if (buffer_cnt != cpu_buffer->cnt) {
> 1544 /* The list was updated, try again. */
> 1545 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> 1546 goto again;
> 1547 }
> 1548
> 1549 tmp = rb_list_head(tmp->next);
> 1550 if (tmp == head)
> 1551 /* The iteration circled back, all is done. */
> 1552 goto out_locked;
> 1553
> 1554 if (!rb_check_links(cpu_buffer, tmp))
> 1555 goto out_locked;
> 1556
> 1557 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> 1558 }
> 1559
> 1560 out_locked:
> 1561 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> 1562 }
> 1563
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [trace:ring-buffer/for-next 2/4] kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting
2024-10-19 15:07 ` Steven Rostedt
@ 2024-10-19 15:14 ` Steven Rostedt
0 siblings, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2024-10-19 15:14 UTC (permalink / raw)
To: kernel test robot; +Cc: Petr Pavlu, oe-kbuild-all
On Sat, 19 Oct 2024 11:07:43 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> > 1527 again:
> > 1528 if (++nr_loops > 3)
> > 1529 return;
> > 1530
> > 1531 raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
> > 1532 head = rb_list_head(cpu_buffer->pages);
> > 1533 if (!rb_check_links(cpu_buffer, head))
> > 1534 goto out_locked;
> > 1535 buffer_cnt = cpu_buffer->cnt;
> > 1536 tmp = head;
> > 1537 raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> > 1538 return;
>
> Bah, an extra "return" was added due to me rebasing the patch incorrectly.
And Petr even sent the proper patch, but in the between time, I was testing
my own rebase that had this bug. Somehow I picked my own rebase to push to
for-next and not the branch where I pulled in Petr's patches.
-- Steve
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-19 15:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-19 14:43 [trace:ring-buffer/for-next 2/4] kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting kernel test robot
2024-10-19 15:07 ` Steven Rostedt
2024-10-19 15:14 ` Steven Rostedt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.