Skip invalid sub-buffers when validating the persistent ring buffer instead of discarding the entire ring buffer. Only skipped buffers are invalidated (cleared). If the cache data in memory fails to be synchronized during a reboot, the persistent ring buffer may become partially corrupted, but other sub-buffers may still contain readable event data. Only discard the subbuffers that are found to be corrupted. Link: https://lore.kernel.org/all/20260520185018.051228084@kernel.org/
Unified · Split
| @@ -370,6 +370,12 @@ static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage) · 369 unmodified lines | ||
| 370 | 370 | return local_read(&bpage->page->commit); |
| 371 | 371 | } |
| 372 | 372 | |
| 373 | +/* Size is determined by what has been committed */ | |
| 374 | +static __always_inline unsigned int rb_page_size(struct buffer_page *bpage) | |
| 375 | +{ | |
| 376 | + return rb_page_commit(bpage) & ~RB_MISSED_MASK; | |
| 377 | +} | |
| 378 | + | |
| 373 | 379 | static void free_buffer_page(struct buffer_page *bpage) |
| 374 | 380 | { |
| 375 | 381 | /* Range pages are not to be freed */ |
| @@ -1762,7 +1768,6 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu, · 1386 unmodified lines | ||
| 1762 | 1768 | unsigned long *subbuf_mask) |
| 1763 | 1769 | { |
| 1764 | 1770 | int subbuf_size = PAGE_SIZE; |
| 1765 | − struct buffer_data_page *subbuf; | |
| 1766 | 1771 | unsigned long buffers_start; |
| 1767 | 1772 | unsigned long buffers_end; |
| 1768 | 1773 | int i; |
| @@ -1770,6 +1775,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu, · 1 unmodified line | ||
| 1770 | 1775 | if (!subbuf_mask) |
| 1771 | 1776 | return false; |
| 1772 | 1777 | |
| 1778 | + if (meta->subbuf_size != PAGE_SIZE) { | |
| 1779 | + pr_info("Ring buffer boot meta [%d] invalid subbuf_size\n", cpu); | |
| 1780 | + return false; | |
| 1781 | + } | |
| 1782 | + | |
| 1773 | 1783 | buffers_start = meta->first_buffer; |
| 1774 | 1784 | buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs); |
| 1775 | 1785 | |
| @@ -1786,11 +1796,12 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu, · 10 unmodified lines | ||
| 1786 | 1796 | return false; |
| 1787 | 1797 | } |
| 1788 | 1798 | |
| 1789 | − subbuf = rb_subbufs_from_meta(meta); | |
| 1790 | − | |
| 1791 | 1799 | bitmap_clear(subbuf_mask, 0, meta->nr_subbufs); |
| 1792 | 1800 | |
| 1793 | − /* Is the meta buffers and the subbufs themselves have correct data? */ | |
| 1801 | + /* | |
| 1802 | + * Ensure the meta::buffers array has correct data. The data in each subbufs | |
| 1803 | + * are checked later in rb_meta_validate_events(). | |
| 1804 | + */ | |
| 1794 | 1805 | for (i = 0; i < meta->nr_subbufs; i++) { |
| 1795 | 1806 | if (meta->buffers[i] < 0 || |
| 1796 | 1807 | meta->buffers[i] >= meta->nr_subbufs) { |
| @@ -1798,18 +1809,12 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu, · 1 unmodified line | ||
| 1798 | 1809 | return false; |
| 1799 | 1810 | } |
| 1800 | 1811 | |
| 1801 | − if ((unsigned)local_read(&subbuf->commit) > subbuf_size) { | |
| 1802 | − pr_info("Ring buffer boot meta [%d] buffer invalid commit\n", cpu); | |
| 1803 | − return false; | |
| 1804 | − } | |
| 1805 | − | |
| 1806 | 1812 | if (test_bit(meta->buffers[i], subbuf_mask)) { |
| 1807 | 1813 | pr_info("Ring buffer boot meta [%d] array has duplicates\n", cpu); |
| 1808 | 1814 | return false; |
| 1809 | 1815 | } |
| 1810 | 1816 | |
| 1811 | 1817 | set_bit(meta->buffers[i], subbuf_mask); |
| 1812 | − subbuf = (void *)subbuf + subbuf_size; | |
| 1813 | 1818 | } |
| 1814 | 1819 | |
| 1815 | 1820 | return true; |
| @@ -1873,13 +1878,22 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu · 57 unmodified lines | ||
| 1873 | 1878 | return events; |
| 1874 | 1879 | } |
| 1875 | 1880 | |
| 1876 | −static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu) | |
| 1881 | +static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu, | |
| 1882 | + struct ring_buffer_cpu_meta *meta) | |
| 1877 | 1883 | { |
| 1878 | 1884 | unsigned long long ts; |
| 1885 | + unsigned long tail; | |
| 1879 | 1886 | u64 delta; |
| 1880 | − int tail; | |
| 1881 | 1887 | |
| 1882 | − tail = local_read(&dpage->commit); | |
| 1888 | + /* | |
| 1889 | + * When a sub-buffer is recovered from a read, the commit value may | |
| 1890 | + * have RB_MISSED_* bits set, as these bits are reset on reuse. | |
| 1891 | + * Even after clearing these bits, a commit value greater than the | |
| 1892 | + * subbuf_size is considered invalid. | |
| 1893 | + */ | |
| 1894 | + tail = local_read(&dpage->commit) & ~RB_MISSED_MASK; | |
| 1895 | + if (tail > meta->subbuf_size - BUF_PAGE_HDR_SIZE) | |
| 1896 | + return -1; | |
| 1883 | 1897 | return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta); |
| 1884 | 1898 | } |
| 1885 | 1899 | |
| @@ -1890,6 +1904,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer) · 4 unmodified lines | ||
| 1890 | 1904 | struct buffer_page *head_page, *orig_head, *orig_reader; |
| 1891 | 1905 | unsigned long entry_bytes = 0; |
| 1892 | 1906 | unsigned long entries = 0; |
| 1907 | + int discarded = 0; | |
| 1893 | 1908 | int ret; |
| 1894 | 1909 | u64 ts; |
| 1895 | 1910 | int i; |
| @@ -1901,14 +1916,19 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer) · 5 unmodified lines | ||
| 1901 | 1916 | orig_reader = cpu_buffer->reader_page; |
| 1902 | 1917 | |
| 1903 | 1918 | /* Do the reader page first */ |
| 1904 | − ret = rb_validate_buffer(orig_reader->page, cpu_buffer->cpu); | |
| 1919 | + ret = rb_validate_buffer(orig_reader->page, cpu_buffer->cpu, meta); | |
| 1905 | 1920 | if (ret < 0) { |
| 1906 | − pr_info("Ring buffer reader page is invalid\n"); | |
| 1907 | − goto invalid; | |
| 1921 | + pr_info("Ring buffer meta [%d] invalid reader page detected\n", | |
| 1922 | + cpu_buffer->cpu); | |
| 1923 | + discarded++; | |
| 1924 | + /* Instead of discard whole ring buffer, discard only this sub-buffer. */ | |
| 1925 | + local_set(&orig_reader->entries, 0); | |
| 1926 | + local_set(&orig_reader->page->commit, 0); | |
| 1927 | + } else { | |
| 1928 | + entries += ret; | |
| 1929 | + entry_bytes += rb_page_size(orig_reader); | |
| 1930 | + local_set(&orig_reader->entries, ret); | |
| 1908 | 1931 | } |
| 1909 | − entries += ret; | |
| 1910 | − entry_bytes += local_read(&orig_reader->page->commit); | |
| 1911 | − local_set(&orig_reader->entries, ret); | |
| 1912 | 1932 | |
| 1913 | 1933 | ts = head_page->page->time_stamp; |
| 1914 | 1934 | |
| @@ -1936,7 +1956,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer) · 21 unmodified lines | ||
| 1936 | 1956 | break; |
| 1937 | 1957 | |
| 1938 | 1958 | /* Stop rewind if the page is invalid. */ |
| 1939 | − ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu); | |
| 1959 | + ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta); | |
| 1940 | 1960 | if (ret < 0) |
| 1941 | 1961 | break; |
| 1942 | 1962 | |
| @@ -1945,7 +1965,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer) · 2 unmodified lines | ||
| 1945 | 1965 | if (ret) |
| 1946 | 1966 | local_inc(&cpu_buffer->pages_touched); |
| 1947 | 1967 | entries += ret; |
| 1948 | − entry_bytes += rb_page_commit(head_page); | |
| 1968 | + entry_bytes += rb_page_size(head_page); | |
| 1949 | 1969 | } |
| 1950 | 1970 | if (i) |
| 1951 | 1971 | pr_info("Ring buffer [%d] rewound %d pages\n", cpu_buffer->cpu, i); |
| @@ -2015,21 +2035,24 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer) · 63 unmodified lines | ||
| 2015 | 2035 | if (head_page == orig_reader) |
| 2016 | 2036 | continue; |
| 2017 | 2037 | |
| 2018 | − ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu); | |
| 2038 | + ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta); | |
| 2019 | 2039 | if (ret < 0) { |
| 2020 | − pr_info("Ring buffer meta [%d] invalid buffer page\n", | |
| 2021 | − cpu_buffer->cpu); | |
| 2022 | − goto invalid; | |
| 2023 | − } | |
| 2024 | − | |
| 2025 | − /* If the buffer has content, update pages_touched */ | |
| 2026 | − if (ret) | |
| 2027 | − local_inc(&cpu_buffer->pages_touched); | |
| 2028 | − | |
| 2029 | − entries += ret; | |
| 2030 | − entry_bytes += local_read(&head_page->page->commit); | |
| 2031 | − local_set(&head_page->entries, ret); | |
| 2040 | + if (!discarded) | |
| 2041 | + pr_info("Ring buffer meta [%d] invalid buffer page detected\n", | |
| 2042 | + cpu_buffer->cpu); | |
| 2043 | + discarded++; | |
| 2044 | + /* Instead of discard whole ring buffer, discard only this sub-buffer. */ | |
| 2045 | + local_set(&head_page->entries, 0); | |
| 2046 | + local_set(&head_page->page->commit, 0); | |
| 2047 | + } else { | |
| 2048 | + /* If the buffer has content, update pages_touched */ | |
| 2049 | + if (ret) | |
| 2050 | + local_inc(&cpu_buffer->pages_touched); | |
| 2032 | 2051 | |
| 2052 | + entries += ret; | |
| 2053 | + entry_bytes += rb_page_size(head_page); | |
| 2054 | + local_set(&head_page->entries, ret); | |
| 2055 | + } | |
| 2033 | 2056 | if (head_page == cpu_buffer->commit_page) |
| 2034 | 2057 | break; |
| 2035 | 2058 | } |
| @@ -2043,7 +2066,10 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer) · 7 unmodified lines | ||
| 2043 | 2066 | local_set(&cpu_buffer->entries, entries); |
| 2044 | 2067 | local_set(&cpu_buffer->entries_bytes, entry_bytes); |
| 2045 | 2068 | |
| 2046 | − pr_info("Ring buffer meta [%d] is from previous boot!\n", cpu_buffer->cpu); | |
| 2069 | + pr_info("Ring buffer meta [%d] is from previous boot!", cpu_buffer->cpu); | |
| 2070 | + if (discarded) | |
| 2071 | + pr_cont(" (%d pages discarded)", discarded); | |
| 2072 | + pr_cont("\n"); | |
| 2047 | 2073 | return; |
| 2048 | 2074 | |
| 2049 | 2075 | invalid: |
| @@ -3330,12 +3356,6 @@ rb_iter_head_event(struct ring_buffer_iter *iter) · 1280 unmodified lines | ||
| 3330 | 3356 | return NULL; |
| 3331 | 3357 | } |
| 3332 | 3358 | |
| 3333 | −/* Size is determined by what has been committed */ | |
| 3334 | −static __always_inline unsigned rb_page_size(struct buffer_page *bpage) | |
| 3335 | −{ | |
| 3336 | − return rb_page_commit(bpage) & ~RB_MISSED_MASK; | |
| 3337 | −} | |
| 3338 | − | |
| 3339 | 3359 | static __always_inline unsigned |
| 3340 | 3360 | rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer) |
| 3341 | 3361 | { |
| @@ -5636,8 +5656,9 @@ __rb_get_reader_page_from_remote(struct ring_buffer_per_cpu *cpu_buffer) · 2294 unmodified lines | ||
| 5636 | 5656 | static struct buffer_page * |
| 5637 | 5657 | __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer) |
| 5638 | 5658 | { |
| 5639 | − struct buffer_page *reader = NULL; | |
| 5659 | + int max_loops = cpu_buffer->ring_meta ? cpu_buffer->nr_pages : 3; | |
| 5640 | 5660 | unsigned long bsize = READ_ONCE(cpu_buffer->buffer->subbuf_size); |
| 5661 | + struct buffer_page *reader = NULL; | |
| 5641 | 5662 | unsigned long overwrite; |
| 5642 | 5663 | unsigned long flags; |
| 5643 | 5664 | int nr_loops = 0; |
| @@ -5649,11 +5670,14 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer) · 5 unmodified lines | ||
| 5649 | 5670 | again: |
| 5650 | 5671 | /* |
| 5651 | 5672 | * This should normally only loop twice. But because the |
| 5652 | − * start of the reader inserts an empty page, it causes | |
| 5653 | − * a case where we will loop three times. There should be no | |
| 5654 | − * reason to loop four times (that I know of). | |
| 5673 | + * start of the reader inserts an empty page, it causes a | |
| 5674 | + * case where we will loop three times. There should be no | |
| 5675 | + * reason to loop four times unless the ring buffer is a | |
| 5676 | + * recovered persistent ring buffer. For persistent ring buffers, | |
| 5677 | + * invalid pages are reset during recovery, so there may be more | |
| 5678 | + * than 3 contiguous pages can be empty, but less than nr_pages. | |
| 5655 | 5679 | */ |
| 5656 | − if (RB_WARN_ON(cpu_buffer, ++nr_loops > 3)) { | |
| 5680 | + if (RB_WARN_ON(cpu_buffer, ++nr_loops > max_loops)) { | |
| 5657 | 5681 | reader = NULL; |
| 5658 | 5682 | goto out; |
| 5659 | 5683 | } |
| @@ -5950,12 +5974,14 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts) · 290 unmodified lines | ||
| 5950 | 5974 | struct ring_buffer_per_cpu *cpu_buffer; |
| 5951 | 5975 | struct ring_buffer_event *event; |
| 5952 | 5976 | int nr_loops = 0; |
| 5977 | + int max_loops; | |
| 5953 | 5978 | |
| 5954 | 5979 | if (ts) |
| 5955 | 5980 | *ts = 0; |
| 5956 | 5981 | |
| 5957 | 5982 | cpu_buffer = iter->cpu_buffer; |
| 5958 | 5983 | buffer = cpu_buffer->buffer; |
| 5984 | + max_loops = cpu_buffer->ring_meta ? cpu_buffer->nr_pages : 3; | |
| 5959 | 5985 | |
| 5960 | 5986 | /* |
| 5961 | 5987 | * Check if someone performed a consuming read to the buffer |
| @@ -5978,7 +6004,7 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts) · 16 unmodified lines | ||
| 5978 | 6004 | * the ring buffer with an active write as the consumer is. |
| 5979 | 6005 | * Do not warn if the three failures is reached. |
| 5980 | 6006 | */ |
| 5981 | − if (++nr_loops > 3) | |
| 6007 | + if (++nr_loops > max_loops) | |
| 5982 | 6008 | return NULL; |
| 5983 | 6009 | |
| 5984 | 6010 | if (rb_per_cpu_empty(cpu_buffer)) |