From patchwork Mon Jul 24 05:40:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zheng Yejian X-Patchwork-Id: 124623 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9010:0:b0:3e4:2afc:c1 with SMTP id l16csp1595806vqg; Sun, 23 Jul 2023 23:03:34 -0700 (PDT) X-Google-Smtp-Source: APBJJlGClRmE9b38dNLmyMwaD4QUrGRD4u6wjQW1/BTBov96xfIjReDXNmvwbFkW7J8hjVCGSomF X-Received: by 2002:aa7:cb4b:0:b0:51a:5a25:6631 with SMTP id w11-20020aa7cb4b000000b0051a5a256631mr7887815edt.3.1690178614025; Sun, 23 Jul 2023 23:03:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690178614; cv=none; d=google.com; s=arc-20160816; b=E8sccszHD7xTGMSVfnDnn/jf2wzRBUQaBRHpbLUuVMHwCVu6ws/MkSUCBq1XiLeIuA aGlP0K3Y8+cPQ1TOhLUSwfey0M/UQkoJkU5+z67bLruJJ51+49IaljIqk2NrPDSxx6aU EG2q9VBm6lfDjdcZHIx9UoR0FM2y7tM7QDw5Piaa7sfuDz5AJYXKcgTc9x3x9N4Wo/SV a9UAIqLsg+9MIoMtUOJjBd8rSnP63l62kTwhWuKbTouB2lbgW7AxL+FRoMtMIEPw2J3B 2lkkKciMjN2EmbHtP3AdmQbb2jfpgOSSmtUC5lh9b8aicvwMlyUPxYqOupFp/tPz/cw8 FISQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=yTJBUlXCtA8WbG3bJjJskM1cXFIIkJmyfrPqzV0w9uc=; fh=JvQJw49X02X93TMWjLelffl+D/lxUFZd/NIUC5xnP/0=; b=Jlps+ztE6GCmE6ZHcHkr84kDXms12sdxLx5sIhS+TfLzX+I/k4YFIpb7LoQYlgPG26 Nz5O/KGiNRXD2KfGrKIV8uF5ft8rFDjdextptbULpWYw+felgs7TenNR1/nM0xE9RP1p U1PvEX6+FVFLp5VCtO79IaOhv1FS8F7WJWd+2P+9A2MvmFOExOVX4p77Jw3ZqyfJ1USK 7eQUMgIIaUyr5xer14nJfNwBgTdY5JhDSDXscPQmWYHMULQjlHMjRUllSGSBhPhlzt8g f1x/OkGIXJVjZSz1XMqKn9L53GI7Xhsi6Bvy5TNYWOF9ojqwQ38aJWzGXMhnPYV83J4D b36g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y4-20020a056402134400b005223ab5506fsi67268edw.546.2023.07.23.23.03.08; Sun, 23 Jul 2023 23:03:34 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229553AbjGXFkk (ORCPT + 99 others); Mon, 24 Jul 2023 01:40:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40336 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229640AbjGXFki (ORCPT ); Mon, 24 Jul 2023 01:40:38 -0400 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 821AF137; Sun, 23 Jul 2023 22:40:35 -0700 (PDT) Received: from dggpeml500012.china.huawei.com (unknown [172.30.72.56]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4R8TWK0pXZzrRjF; Mon, 24 Jul 2023 13:39:41 +0800 (CST) Received: from localhost.localdomain (10.67.175.61) by dggpeml500012.china.huawei.com (7.185.36.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Mon, 24 Jul 2023 13:40:31 +0800 From: Zheng Yejian To: , , CC: , , Subject: [PATCH] ring-buffer: Fix wrong stat of cpu_buffer->read Date: Mon, 24 Jul 2023 13:40:40 +0800 Message-ID: <20230724054040.3489499-1-zhengyejian1@huawei.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Originating-IP: [10.67.175.61] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml500012.china.huawei.com (7.185.36.15) X-CFilter-Loop: Reflected X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772280730318777333 X-GMAIL-MSGID: 1772280730318777333 When pages are removed in rb_remove_pages(), 'cpu_buffer->read' is set to 0 in order to make sure any read iterators reset themselves. However, this will mess 'entries' stating, see following steps: # cd /sys/kernel/tracing/ # 1. Enlarge ring buffer prepare for later reducing: # echo 20 > per_cpu/cpu0/buffer_size_kb # 2. Write a log into ring buffer of cpu0: # taskset -c 0 echo "hello1" > trace_marker # 3. Read the log: # cat per_cpu/cpu0/trace_pipe <...>-332 [000] ..... 62.406844: tracing_mark_write: hello1 # 4. Stop reading and see the stats, now 0 entries, and 1 event readed: # cat per_cpu/cpu0/stats entries: 0 [...] read events: 1 # 5. Reduce the ring buffer # echo 7 > per_cpu/cpu0/buffer_size_kb # 6. Now entries became unexpected 1 because actually no entries!!! # cat per_cpu/cpu0/stats entries: 1 [...] read events: 0 To fix it, introduce 'page_removed' field to count total removed pages since last reset, then use it to let read iterators reset themselves instead of changing the 'read' pointer. Fixes: 83f40318dab0 ("ring-buffer: Make removal of ring buffer pages atomic") Signed-off-by: Zheng Yejian --- kernel/trace/ring_buffer.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index de061dd47313..46b4a3c7c3bf 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -523,6 +523,8 @@ struct ring_buffer_per_cpu { rb_time_t before_stamp; u64 event_stamp[MAX_NEST]; u64 read_stamp; + /* pages removed since last reset */ + unsigned long pages_removed; /* ring buffer pages to update, > 0 to add, < 0 to remove */ long nr_pages_to_update; struct list_head new_pages; /* new pages to add */ @@ -559,6 +561,7 @@ struct ring_buffer_iter { struct buffer_page *head_page; struct buffer_page *cache_reader_page; unsigned long cache_read; + unsigned long cache_pages_removed; u64 read_stamp; u64 page_stamp; struct ring_buffer_event *event; @@ -1957,6 +1960,8 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages) to_remove = rb_list_head(to_remove)->next; head_bit |= (unsigned long)to_remove & RB_PAGE_HEAD; } + /* Read iterators need to reset themselves when some pages removed */ + cpu_buffer->pages_removed += nr_removed; next_page = rb_list_head(to_remove)->next; @@ -1978,12 +1983,6 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages) cpu_buffer->head_page = list_entry(next_page, struct buffer_page, list); - /* - * change read pointer to make sure any read iterators reset - * themselves - */ - cpu_buffer->read = 0; - /* pages are removed, resume tracing and then free the pages */ atomic_dec(&cpu_buffer->record_disabled); raw_spin_unlock_irq(&cpu_buffer->reader_lock); @@ -4395,6 +4394,7 @@ static void rb_iter_reset(struct ring_buffer_iter *iter) iter->cache_reader_page = iter->head_page; iter->cache_read = cpu_buffer->read; + iter->cache_pages_removed = cpu_buffer->pages_removed; if (iter->head) { iter->read_stamp = cpu_buffer->read_stamp; @@ -4849,12 +4849,13 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts) buffer = cpu_buffer->buffer; /* - * Check if someone performed a consuming read to - * the buffer. A consuming read invalidates the iterator - * and we need to reset the iterator in this case. + * Check if someone performed a consuming read to the buffer + * or removed some pages from the buffer. In these cases, + * iterator was invalidated and we need to reset it. */ if (unlikely(iter->cache_read != cpu_buffer->read || - iter->cache_reader_page != cpu_buffer->reader_page)) + iter->cache_reader_page != cpu_buffer->reader_page || + iter->cache_pages_removed != cpu_buffer->pages_removed)) rb_iter_reset(iter); again: @@ -5298,6 +5299,7 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer) cpu_buffer->last_overrun = 0; rb_head_page_activate(cpu_buffer); + cpu_buffer->pages_removed = 0; } /* Must have disabled the cpu buffer then done a synchronize_rcu */