Re: [bug report] ring-buffer: Limit time with disabled interrupts in rb_check_pages()

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 22 Oct 2024 12:09:47 +0300
Dan Carpenter <dan.carpenter@linaro.org> wrote:

> Hello Petr Pavlu,

It's not Petr's fault, but mine.

> 
> Commit 1f1c2bc9d075 ("ring-buffer: Limit time with disabled
> interrupts in rb_check_pages()") from Jul 15, 2024 (linux-next),
> leads to the following Smatch static checker warning:
> 

>     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;
>                          ^^^^^^
> You probably intended to delete this return?

He sent two patches where the second patch was a fix to mainline. I
asked him to reverse the order so I could send the fix to Linus, in
which he did. In the mean time I switched the order myself for testing
and broke it with this mistake. When he sent the update I took his
patches but accidentally tested and pushed the broken branch to
linux-next.

When Linus took the fix, Stephen Rothwell reported a bad merge between
next and Linus's tree. That's when I realized I had pushed the wrong
branch.

This is the broken branch. I already updated my for-next branch with
the correct commits and when Stephen syncs the linux-next tree, it
should be fixed.

-- 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 }
> 
> regards,
> dan carpenter