From: John Kacur <jkacur@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Clark Williams <williams@redhat.com>,
Steven Rostedt <rostedt@goodmis.org>,
lkml <linux-kernel@vger.kernel.org>,
rt-users <linux-rt-users@vger.kernel.org>
Subject: Re: [PATCH:resend] rt: Remove CONFIG_STACK_TRACER from DEBUG_COUNT, and fix reminder block
Date: Wed, 28 Apr 2010 21:22:02 +0200 [thread overview]
Message-ID: <z2o520f0cf11004281222h4c1cecb6mf41ab89d66ddfcbf@mail.gmail.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1004272205380.2951@localhost.localdomain>
On Tue, Apr 27, 2010 at 10:39 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 27 Apr 2010, John Kacur wrote:
>
>> This fix does two things.
>
> This changelog does suck in more than two ways.
>
>> 1. It reverts commit 3e39399ef4a742d994570488994be93ef17d4ef5.
>> - The above commit added back a warning about CONFIG_STACK_TRACER
>> NOT because the warning was necessary but to fix-up a problem where you could
>> get an empty REMINDER block in the demg.
>
> The CONFIG_STACK_TRACER printk has been there before and got dropped
> for whatever reason, but the DEBUG_COUNT accounting of
> CONFIG_STACK_TRACER still remained.
>
> So the conclusion that the printk needs to be added back is _NOT_
> completely unreasonable.
>
>> However, CONFIG_STACK_TRACER really should not be in the reminder block.
>> Even when configured-in it is not enabled, and thus has neglible impact.
>
> That argument is bogus as it applies to most of the tracing related
> config options.
Not quite. Some of the tracing options are believed to have an impact
even when not enabled.
In particular irqsoff and preemptoff still call a function. Not sure
how minimal the impact is, but
it isn't zero.
>
>> 2. It removes CONFIG_STACK_TRACER from DEBUG_COUNT which is the proper fix
>> for the empty REMINDER block problem.
>
> It's the proper fix if the warning is not justified, which is not
> clear at all. At least not from your changelog.
>
> I do not mind the patch per se, but
>
> 1) the changelog should simply say:
>
> rt: Remove CONFIG_STACK_TRACER from REMINDER block
>
> Reason: It does not matter because ....
>
> 2) the patch should cover _all_ tracing options which have no
> considerable latency impact when just compiled in and not default
> enabled.
>
Okay, you make some good points, although I'm having a hard time
finding other CONFIGS to safely cut.
Respinning the patch. Thanks for your reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
prev parent reply other threads:[~2010-04-28 19:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-27 19:25 [PATCH:resend] rt: Remove CONFIG_STACK_TRACER from DEBUG_COUNT, and fix reminder block John Kacur
2010-04-27 19:34 ` Clark Williams
2010-04-27 20:39 ` Thomas Gleixner
2010-04-28 19:22 ` John Kacur [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=z2o520f0cf11004281222h4c1cecb6mf41ab89d66ddfcbf@mail.gmail.com \
--to=jkacur@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=williams@redhat.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).