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

On Tue, 22 Oct 2024 12:09:47 +0300
Dan Carpenter [off-list ref] 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