From nobody Wed Dec 17 11:26:04 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7D546CDB482 for ; Thu, 19 Oct 2023 13:25:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345909AbjJSNZx (ORCPT ); Thu, 19 Oct 2023 09:25:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42870 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345506AbjJSNZv (ORCPT ); Thu, 19 Oct 2023 09:25:51 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 282B8112 for ; Thu, 19 Oct 2023 06:25:49 -0700 (PDT) From: John Ogness DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1697721947; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=qqu0c4veMedQA7+QrPVXf4u8RrBwewGwZ9vXhntbfUc=; b=CoR28aeP/weSHSftc7F9y/aWGi72I7XcWrp7NTM3WuzAxizJ2BRBeu88Vf2f88SEj1PZVX VjLnQvw2kJ7AnnRRkFt/IeJZayzF84Vvhyd/h31OsTxdWCtO6vSvDwktf96U2X+bAJV5ip S9sXOGQirVad9vRI8AirvsTR+W64/MCN60M+hvAygTF2sed1/PxklLtrRuKfymBrMP/e4h KpO+NUnq/dSwBfXJWkLORxHI3p4TQWRMOamWQlTKk83MrIcOaQY5qHZk6Dg5zuEHXrUAby xo3KH6x5N3A50jC4if77eQ3DjPcIMisutBhB+oJqJKZeipILT8AyuZizI9gnzQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1697721947; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=qqu0c4veMedQA7+QrPVXf4u8RrBwewGwZ9vXhntbfUc=; b=ffPn3lxlLolbHhH1XcsbSUB5VjBoSDY5LSjPCCvKXumAinS3fhLBcmmMHlmK1oCDi1r6PS Aw8RorsZCBnfoUDQ== To: Petr Mladek Cc: Sergey Senozhatsky , Steven Rostedt , Thomas Gleixner , linux-kernel@vger.kernel.org, Mukesh Ojha , Chunlei Wang Subject: [RFC PATCH printk v1] printk: ringbuffer: Do not skip non-finalized with prb_next_seq() Date: Thu, 19 Oct 2023 15:31:45 +0206 Message-Id: <20231019132545.1190490-1-john.ogness@linutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Commit f244b4dc53e5 ("printk: ringbuffer: Improve prb_next_seq() performance") introduced an optimization for prb_next_seq() by using best-effort to track recently finalized records. However, the order of finalization does not necessarily match the order of the records. This can lead to prb_next_seq() returning higher than desired sequence numbers, which results in the reader skipping over records that are not yet finalized. From the reader's perspective it results in messages never being seen. Rather than simply tracking recently finalized records, force the committing writer to read records and increment the last "contiguous block" of finalized records. In order to do this, the sequence number instead of ID must be stored because ID's cannot be directly compared. For 32bit systems, only the lower 32 bits of the sequence number are stored. When reading the value, it is expanded to the full 64bit sequence number by folding in the value returned by prb_first_seq(). Fixes: f244b4dc53e5 ("printk: ringbuffer: Improve prb_next_seq() performanc= e") Signed-off-by: John Ogness --- kernel/printk/printk_ringbuffer.c | 114 +++++++++++++++++++++--------- kernel/printk/printk_ringbuffer.h | 4 +- 2 files changed, 82 insertions(+), 36 deletions(-) diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringb= uffer.c index fde338606ce8..f9fbfa21c5b2 100644 --- a/kernel/printk/printk_ringbuffer.c +++ b/kernel/printk/printk_ringbuffer.c @@ -1441,20 +1441,83 @@ bool prb_reserve_in_last(struct prb_reserved_entry = *e, struct printk_ringbuffer return false; } =20 +#ifdef CONFIG_64BIT + +#define __u64seq_to_ulseq(u64seq) (u64seq) +#define __ulseq_to_u64seq(ulseq) (ulseq) + +#else /* CONFIG_64BIT */ + +static u64 prb_first_seq(struct printk_ringbuffer *rb); + +#define __u64seq_to_ulseq(u64seq) ((u32)u64seq) +static inline u64 __ulseq_to_u64seq(u32 ulseq) +{ + u64 rb_first_seq =3D prb_first_seq(prb); + u64 seq; + + /* + * The provided sequence is only the lower 32 bits of the ringbuffer + * sequence. It needs to be expanded to 64bit. Get the first sequence + * number from the ringbuffer and fold it. + */ + seq =3D rb_first_seq - ((u32)rb_first_seq - ulseq); + + return seq; +} + +#endif /* CONFIG_64BIT */ + +static u64 desc_last_finalized_seq(struct prb_desc_ring *desc_ring) +{ + unsigned long ulseq =3D atomic_long_read(&desc_ring->last_finalized_seq); + + return __ulseq_to_u64seq(ulseq); +} + +static bool _prb_read_valid(struct printk_ringbuffer *rb, u64 *seq, + struct printk_record *r, unsigned int *line_count); + +/* + * Check if there are records directly following @last_finalized_seq that = are + * finalized. If so, update @last_finalized_seq to the latest of these + * records. It is not allowed to skip over records that are not yet finali= zed. + */ +static void desc_update_last_finalized(struct printk_ringbuffer *rb) +{ + struct prb_desc_ring *desc_ring =3D &rb->desc_ring; + u64 prev_seq =3D desc_last_finalized_seq(desc_ring); + u64 seq =3D prev_seq; + + while (_prb_read_valid(rb, &seq, NULL, NULL)) { + unsigned long oldval =3D __u64seq_to_ulseq(prev_seq); + unsigned long newval =3D __u64seq_to_ulseq(seq); + + if (atomic_long_try_cmpxchg_relaxed(&desc_ring->last_finalized_seq, + &oldval, newval)) { + prev_seq =3D seq; + } else { + prev_seq =3D __ulseq_to_u64seq(oldval); + } + + seq =3D prev_seq + 1; + } +} + /* * Attempt to finalize a specified descriptor. If this fails, the descript= or * is either already final or it will finalize itself when the writer comm= its. */ -static void desc_make_final(struct prb_desc_ring *desc_ring, unsigned long= id) +static void desc_make_final(struct printk_ringbuffer *rb, unsigned long id) { + struct prb_desc_ring *desc_ring =3D &rb->desc_ring; unsigned long prev_state_val =3D DESC_SV(id, desc_committed); struct prb_desc *d =3D to_desc(desc_ring, id); =20 atomic_long_cmpxchg_relaxed(&d->state_var, prev_state_val, DESC_SV(id, desc_finalized)); /* LMM(desc_make_final:A) */ =20 - /* Best effort to remember the last finalized @id. */ - atomic_long_set(&desc_ring->last_finalized_id, id); + desc_update_last_finalized(rb); } =20 /** @@ -1550,7 +1613,7 @@ bool prb_reserve(struct prb_reserved_entry *e, struct= printk_ringbuffer *rb, * readers. (For seq=3D=3D0 there is no previous descriptor.) */ if (info->seq > 0) - desc_make_final(desc_ring, DESC_ID(id - 1)); + desc_make_final(rb, DESC_ID(id - 1)); =20 r->text_buf =3D data_alloc(rb, r->text_buf_size, &d->text_blk_lpos, id); /* If text data allocation fails, a data-less record is committed. */ @@ -1643,7 +1706,7 @@ void prb_commit(struct prb_reserved_entry *e) */ head_id =3D atomic_long_read(&desc_ring->head_id); /* LMM(prb_commit:A) */ if (head_id !=3D e->id) - desc_make_final(desc_ring, e->id); + desc_make_final(e->rb, e->id); } =20 /** @@ -1663,12 +1726,9 @@ void prb_commit(struct prb_reserved_entry *e) */ void prb_final_commit(struct prb_reserved_entry *e) { - struct prb_desc_ring *desc_ring =3D &e->rb->desc_ring; - _prb_commit(e, desc_finalized); =20 - /* Best effort to remember the last finalized @id. */ - atomic_long_set(&desc_ring->last_finalized_id, e->id); + desc_update_last_finalized(e->rb); } =20 /* @@ -2017,33 +2077,19 @@ u64 prb_first_valid_seq(struct printk_ringbuffer *r= b) u64 prb_next_seq(struct printk_ringbuffer *rb) { struct prb_desc_ring *desc_ring =3D &rb->desc_ring; - enum desc_state d_state; - unsigned long id; u64 seq; =20 - /* Check if the cached @id still points to a valid @seq. */ - id =3D atomic_long_read(&desc_ring->last_finalized_id); - d_state =3D desc_read(desc_ring, id, NULL, &seq, NULL); + seq =3D __ulseq_to_u64seq(atomic_long_read(&desc_ring->last_finalized_seq= )); =20 - if (d_state =3D=3D desc_finalized || d_state =3D=3D desc_reusable) { - /* - * Begin searching after the last finalized record. - * - * On 0, the search must begin at 0 because of hack#2 - * of the bootstrapping phase it is not known if a - * record at index 0 exists. - */ - if (seq !=3D 0) - seq++; - } else { - /* - * The information about the last finalized sequence number - * has gone. It should happen only when there is a flood of - * new messages and the ringbuffer is rapidly recycled. - * Give up and start from the beginning. - */ - seq =3D 0; - } + /* + * Begin searching after the last finalized record. + * + * On 0, the search must begin at 0 because of hack#2 + * of the bootstrapping phase it is not known if a + * record at index 0 exists. + */ + if (seq !=3D 0) + seq++; =20 /* * The information about the last finalized @seq might be inaccurate. @@ -2085,7 +2131,7 @@ void prb_init(struct printk_ringbuffer *rb, rb->desc_ring.infos =3D infos; atomic_long_set(&rb->desc_ring.head_id, DESC0_ID(descbits)); atomic_long_set(&rb->desc_ring.tail_id, DESC0_ID(descbits)); - atomic_long_set(&rb->desc_ring.last_finalized_id, DESC0_ID(descbits)); + atomic_long_set(&rb->desc_ring.last_finalized_seq, 0); =20 rb->text_data_ring.size_bits =3D textbits; rb->text_data_ring.data =3D text_buf; diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringb= uffer.h index 18cd25e489b8..3374a5a3303e 100644 --- a/kernel/printk/printk_ringbuffer.h +++ b/kernel/printk/printk_ringbuffer.h @@ -75,7 +75,7 @@ struct prb_desc_ring { struct printk_info *infos; atomic_long_t head_id; atomic_long_t tail_id; - atomic_long_t last_finalized_id; + atomic_long_t last_finalized_seq; }; =20 /* @@ -259,7 +259,7 @@ static struct printk_ringbuffer name =3D { \ .infos =3D &_##name##_infos[0], \ .head_id =3D ATOMIC_INIT(DESC0_ID(descbits)), \ .tail_id =3D ATOMIC_INIT(DESC0_ID(descbits)), \ - .last_finalized_id =3D ATOMIC_INIT(DESC0_ID(descbits)), \ + .last_finalized_seq =3D ATOMIC_INIT(0), \ }, \ .text_data_ring =3D { \ .size_bits =3D (avgtextbits) + (descbits), \ base-commit: a26f18f291f6c2eac6dac9faa12cc68fd1da5865 --=20 2.39.2