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;
Date:   Mon, 26 Sep 2022 18:18:33 +0900
From:   Sergey Senozhatsky <senozhatsky@chromium.org>
To:     John Ogness <john.ogness@linutronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@chromium.org>,
        Petr Mladek <pmladek@suse.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH printk 18/18] printk: Handle dropped message smarter
Message-ID: <YzFuaUq352geBXcE@google.com>
References: <20220924000454.3319186-1-john.ogness@linutronix.de>
 <20220924000454.3319186-19-john.ogness@linutronix.de>
 <YzEoYPSC5Qf2aL92@google.com>
 <87leq6d0zn.fsf@jogness.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87leq6d0zn.fsf@jogness.linutronix.de>
Precedence: bulk

On (22/09/26 10:00), John Ogness wrote:
> > Oh, hmm. This does not look to me as a simplification. Quite
> > the opposite, moving cons_text_buf::text pointer to point to
> > cons_text_buf::text - strlen("... dropped messages...") looks
> > somewhat fragile.
> 
> It relies on @ext_text and @text being packed together, which yes, may
> be fragile.

Right, and this assumption can be broken by both internal and external
sources: new gcc/clang plugins, config options, etc.

> As an alternative we could memcpy the message text (@text)
> to the end of the dropped message text. There would be enough room.
> 
> Generally speaking, the dropped text will be less text to copy. But
> since dropped messages are rare anyway, it might be worth copying more
> data so that the code is not fragile. It would also allow us to remove
> the __no_randomize_layout in "struct cons_text_buf".

Agreed.

> If the end of cons_print_dropped was changed to:
> 
>         memcpy(txtbuf->ext_text + len, txtbuf->text, desc->len);
>         desc->len += len;
>         desc->outbuf = txtbuf->ext_text;
> 
> Would that be OK for you?

Yes, this looks solid (nothing should be able to break it and cause
mem corruptions).