Re: [PATCH] watchdog: Quiet down the boot messages

[Joe Perches]

On Thu, 2012-06-07 at 22:15 -0400, Don Zickus wrote:
> A bunch of bugzillas have complained how noisy the nmi_watchdog is during
> boot-up especially with its expected failure cases (like virt and bios
> resource contention).

Hi Don, this seems nicer.

Just some trivial comments below:

> This is my attempt to quiet them down and keep it less confusing for the end
> user.  What I did is print the message for cpu0 and save it for future
> comparisions.

comparisons

[]
> After the change, it is simlified to:

simplified

> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
[]
> @@ -377,6 +377,14 @@ static int watchdog_nmi_enable(int cpu)
>  	struct perf_event_attr *wd_attr;
>  	struct perf_event *event = per_cpu(watchdog_ev, cpu);
>  
> +	/*
> +	 * People like the simple clean cpu node info
> +	 * on boot.  Simplify the noise from the watchdog
> +	 * by only printing messages that are different than
> +	 * what cpu0 displayed
> +	 */

This comment could be shortened by a line
	/*
	 * People like simple and clean cpu node info on boot.
	 * Reduce the watchdog noise by only printing messages
	 * that are different from what cpu0 displayed.
	 */

> +	static unsigned long err0 = 0;

Strictly, this doesn't need initialization.
I think the err0 name is unclear.  Maybe cpu0_err instead;

I think the use of !cpu and cpu is unclear.
It's still a cpu, just index 0.
!cpu should be cpu == 0 and
cpu should be cpu != 0 too.


> @@ -390,11 +398,21 @@ static int watchdog_nmi_enable(int cpu)
>  
>  	/* Try to register using hardware perf events */
>  	event = perf_event_create_kernel_counter(wd_attr, cpu, NULL, watchdog_overflow_callback, NULL);
> +
> +	/* save cpu0 error for future comparision */
> +	if (!cpu)
> +		err0 = (IS_ERR(event) ? PTR_ERR(event) : 0);

	if (cpu == 0 && IS_ERR(event))
		cpu0_err = PTR_ERR(event);

> +
>  	if (!IS_ERR(event)) {
> -		pr_info("enabled, takes one hw-pmu counter.\n");
> +		/* only print for cpu0 or different than cpu0 */

	or if

> +		if (!cpu || err0)

		if (cpu == 0 || cpu0_err)

> +			pr_info("enabled, takes one hw-pmu counter.\n");
>  		goto out_save;
>  	}
>  
> +	/* skip displaying the same error again */
> +	if ((PTR_ERR(event) == err0) && cpu)

	if (cpu > 0 && PTR_ERR(event))
> +		return PTR_ERR(event);
>  
>  	/* vary the KERN level based on the returned errno */
>  	if (PTR_ERR(event) == -EOPNOTSUPP)

cheers, Joe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/