LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] livepatch: Use static buffer for debugging messages under rq lock
Date: Tue, 7 May 2019 08:19:07 -0500	[thread overview]
Message-ID: <20190507131907.ep5g2hi4hp2lvy3d@treble> (raw)
In-Reply-To: <20190507115029.54qxbsd4vsouwvjo@pathway.suse.cz>

On Tue, May 07, 2019 at 01:50:29PM +0200, Petr Mladek wrote:
> On Mon 2019-05-06 19:43:19, Josh Poimboeuf wrote:
> > On Tue, Apr 30, 2019 at 11:10:49AM +0200, Petr Mladek wrote:
> > > klp_try_switch_task() is called under klp_mutex. The buffer for
> > > debugging messages might be static.
> > 
> > The patch description is missing a "why" (presumably to reduce stack
> > usage).
> 
> Exactly. I thought that it was obvious. But I am infected by printk
> code where line buffers are 1k and nobody wants them on the stack.
> 
> 128bytes in klp_try_switch_task() context are acceptable but
> it is still rather big buffer.
> 
> OK, what about the following commit message?
> 
> "klp_try_switch_task() is called under klp_mutex. The buffer for
> debugging messages might be static to reduce stack usage."

It's better to use imperative language.  It would also be good to
reverse the order of the wording by starting with the problem.

Something like:

The err_buf array uses 128 bytes of stack space.  Move it off the stack
by making it static.  It's safe to use a shared buffer because
klp_try_switch_task() is called under klp_mutex.

-- 
Josh

      reply	other threads:[~2019-05-07 13:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30  9:10 [PATCH v2 0/2] livepatch: Clean up of reliable stacktrace warnings Petr Mladek
2019-04-30  9:10 ` [PATCH v2 1/2] livepatch: Remove duplicate warning about missing reliable stacktrace support Petr Mladek
2019-04-30 11:30   ` Miroslav Benes
2019-04-30 11:56   ` Kamalesh Babulal
2019-05-07  0:40   ` Josh Poimboeuf
2019-05-07  1:43     ` Josh Poimboeuf
2019-05-07 11:29       ` Petr Mladek
2019-05-07 12:03         ` Josh Poimboeuf
2019-05-07 14:28           ` Petr Mladek
2019-05-07 21:24             ` Josh Poimboeuf
2019-05-09  7:24               ` Miroslav Benes
2019-05-13 10:43               ` Petr Mladek
2019-05-13 16:46                 ` Josh Poimboeuf
2019-05-07  8:58     ` Miroslav Benes
2019-04-30  9:10 ` [PATCH v2 2/2] livepatch: Use static buffer for debugging messages under rq lock Petr Mladek
2019-04-30 11:32   ` Miroslav Benes
2019-04-30 12:30   ` Kamalesh Babulal
2019-05-07  0:43   ` Josh Poimboeuf
2019-05-07 11:50     ` Petr Mladek
2019-05-07 13:19       ` Josh Poimboeuf [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190507131907.ep5g2hi4hp2lvy3d@treble \
    --to=jpoimboe@redhat.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=kamalesh@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).