On Mon, Apr 28, 2014 at 05:25:08PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
>
> Currently, torture_kthread_stopping() prints only the name of the
> kthread that is stopping, which can be unedifying. This commit therefore
> adds "Stopping" to make things more evident.
>
> Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
Feedback below.
> kernel/torture.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/torture.c b/kernel/torture.c
> index acc9afc2f26e..f329848c3eee 100644
> --- a/kernel/torture.c
> +++ b/kernel/torture.c
> @@ -674,8 +674,11 @@ EXPORT_SYMBOL_GPL(torture_must_stop_irq);
> */
> void torture_kthread_stopping(char *title)
> {
> + char buf[128];
> +
> + snprintf(buf, sizeof(buf), "Stopping %s", title);
> if (verbose)
> - VERBOSE_TOROUT_STRING(title);
> + VERBOSE_TOROUT_STRING(buf);
This seems like a case where the macro has led to poorer code; rather
than using sprintf into a temporary buffer, this should just print.
Please consider fixing the output macros to allow formats, as the pr_*
macros do.
Also, why do you need "if (verbose)" if the name of the macro has
VERBOSE_ in it; shouldn't that mean it checks verbosity itself?
(Another good reason not to create a unique verbosity level mechanism,
and to use the kernel mechanisms instead.)
> while (!kthread_should_stop()) {
> torture_shutdown_absorb(title);
> schedule_timeout_uninterruptible(1);
> --
> 1.8.1.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
