Received: by 2002:a05:6358:16cc:b0:ea:6187:17c9 with SMTP id r12csp12051967rwl; Tue, 3 Jan 2023 08:21:42 -0800 (PST) X-Google-Smtp-Source: AMrXdXuuxZbFzCGUTGRA2jntXXtZMRa/dgWQwvIa/AiBpdI04i8vaGhyD/92wNzfRzlDqefR81SR X-Received: by 2002:a05:6402:1654:b0:461:78ef:56f2 with SMTP id s20-20020a056402165400b0046178ef56f2mr38325305edx.34.1672762902306; Tue, 03 Jan 2023 08:21:42 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672762902; cv=none; d=google.com; s=arc-20160816; b=NgiwgehYCXZ/jGKUp6Ge3T42XYyQI/xthUu218fDbbVGHOOfkxY3yfaMHM2tqMXB31 fLXhN4MhD3pqK6cfq8dkFbn74bByUhya9GKJl8jb6J4qnvzYf4iqN6iXFyVSc0sSJUB7 olOh6UUxDo4LG/v97biT+LDlUmchIUe3jVzVDUlu5rjaYNSM5+JiXA0vnX0M3Ttzpqeg t0H2YUhYEsfYU6EXB/K6Df1EWeqZHNdu+1J87LCQ5p4pWhfAUaNonwYhcy22GS5XnBDC +Jege74C0bHB5HjjYg7W8XL3JMSfWsGa0wRszWkJpF3TlASY/Y0DjxD9cScn566yDrcO h7HA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=0DOInQfAlC9sXPHhOjtzrZxiECCDaUvA4gDS+HcGDDE=; b=UbcxfOQObQaJajxse1bNHunEQP4vBlDSxxUUFi0CjWJB+OKDI4Gz3fKpLW/gBtLVZ2 lG9K05nroJAS8ZnqZJ969RZbGhDFnmxL7jnvJqRJ91gxlySqCaMDZ/ttUYe6E6C8hFv3 wiQHWzsIRFPQHkbZ2zwMct9RTk3vwq3QVyKUr4AS3iAIJ/iZ+IqaF+uVa1a1fVhF4ywy ZTdgSsg9Oueq1wKbbz7sVP4DLVO0WzZvDPs4txp6mdc1gzlLhtmr7oBh+3//5C29Wo9P 3cNEUsK0dQdrq7fE4+GXMlFGVaahgI8gseN+tDPpY65YCEsK+wbinFHbUAE9/YMgYB/o m05A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=Aemb67Ym; 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=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id qw35-20020a1709066a2300b00839f873bf75si28520037ejc.511.2023.01.03.08.21.27; Tue, 03 Jan 2023 08:21:42 -0800 (PST) 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; dkim=pass header.i=@suse.com header.s=susede1 header.b=Aemb67Ym; 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=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233317AbjACPzY (ORCPT + 60 others); Tue, 3 Jan 2023 10:55:24 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233158AbjACPzW (ORCPT ); Tue, 3 Jan 2023 10:55:22 -0500 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EFC9A11C10 for ; Tue, 3 Jan 2023 07:55:20 -0800 (PST) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id AF78E34431; Tue, 3 Jan 2023 15:55:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1672761319; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=0DOInQfAlC9sXPHhOjtzrZxiECCDaUvA4gDS+HcGDDE=; b=Aemb67YmrlyYVarpKx3qttcf3Rd9VAYg0mWCAPM6RlbC9r/ZdMe0pTsoUvBqviRpM1K7bi G1ky0RqS9Jb9CfcWSSF0CejvKioHXC3QB70Buj8KgBbC200YmJ5cl5NysmHx3y4mitXb3o aAproWjZ7AflFXdJ+FGpPoO6kUsLmxo= Received: from suse.cz (unknown [10.100.201.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 6D08C2C141; Tue, 3 Jan 2023 15:55:19 +0000 (UTC) Date: Tue, 3 Jan 2023 16:55:19 +0100 From: Petr Mladek To: John Ogness Cc: Sergey Senozhatsky , Steven Rostedt , Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH printk v3 5/6] printk: introduce console_get_next_message() and console_message Message-ID: References: <20221221202704.857925-1-john.ogness@linutronix.de> <20221221202704.857925-6-john.ogness@linutronix.de> <87bknva1jg.fsf@jogness.linutronix.de> <87ilhnd5te.fsf@jogness.linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ilhnd5te.fsf@jogness.linutronix.de> X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS 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 On Tue 2023-01-03 16:03:17, John Ogness wrote: > On 2023-01-03, Petr Mladek wrote: > > On Thu 2022-12-22 16:47:39, John Ogness wrote: > >> On 2022-12-21, John Ogness wrote: > >> > +static bool console_emit_next_record(struct console *con, bool *handover, int cookie) > >> > +{ > >> > + bool is_extended = console_srcu_read_flags(con) & CON_EXTENDED; > >> > + static char dropped_text[DROPPED_TEXT_MAX]; > >> > + static struct console_buffers cbufs; > >> > + static struct console_message cmsg = { > >> > + .cbufs = &cbufs, > >> > + }; > >> > >> @cmsg should not be static. The whole point of the console_message > >> wrapper struct is so that it can sit on the stack. > > > > Well, it might actually be better to keep it static for now. > > It always points to static struct console_buffers cbufs anyway. > > > > It would make sense to have it on stack only when it uses > > different buffers. > > I think we should avoid making things static when it serves no purpose. I agree. Well, I still feel that both structures are tightly connected and there should be 1:1 relation. See below. > > Which brings the question. Does it makes sense to use > > the same buffers by different struct console_message? > > Will it be safe in any situation? > > > > I did not want to complicate it yesterday. I think that > > I have already proposed this. But this brings back > > the question whether it makes sense to have two structures > > at all. > > > > I still think that it would be easier and even more safe > > to put everything into struct console_message. > > > > I mean to have: > > > > struct console_message { > > char buf[CONSOLE_EXT_LOG_MAX]; > > char scratch_buf[LOG_LINE_MAX]; > > unsigned int len; > > u64 seq; > > unsigned long dropped; > > }; > > The current atomic console proposal allocates 1x cbuf per-cpu and 4x > meta-data per-cpu. Different contexts of a cpu will have different > meta-data, but all the contexts of a cpu will share the same cbuf. > > If cbufs become embedded in cmsg, then we would allocate 1x cmsg > per-cpu. But the atomic consoles would still need their own 4x per-cpu > meta-data. Do we really need 4x the meta data? The metadata describe the state of the buffer. Using the buffer in one context invalidates the metadata in the other context. By other words, if one context reuses the buffer it might also reuse the metadata. We just need to inform the original context that it would need to break or re-start the operation. But we would need to do this anyway. This is my intuitive view. It is possible that I miss some important detail. > When looking at the proposal code, it looks wrong to have meta-data > fields in the cmsg struct that are not being used. But maybe that is > acceptable during the "transition phase" until all legacy consoles are > gone. I am not sure what you mean by "not being used". When I try to find "struct console_buffers" with cscope than I see it in: + struct console_message definition: + part of the structure + console_prepend_dropped() + console_get_next_message() + both read the pointer from the given struct message + console_emit_next_record() + static definition + the pointer is stored into struct console_message In all situations, struct console_message is needed when struct console_buffers is manipulated. By other words, the metadata are always needed when the buffers are used. > For v4 I will drop the console_buffers struct. I will use your > suggestion. Please, do not do it just to make me happy. My intention is to keep things simple. And keeping the two structures synced needs an extra code. If you are sure that the separation will really be needed in the future then feel free to keep the two structures. Best Regards, Petr