Re: [trace:ring-buffer/for-next 2/4] kernel/trace/ring_buffer.c:1540 rb_check_pages() warn: inconsistent indenting

[Steven Rostedt <rostedt@goodmis.org>]

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	
>