linux-kernel - Re: [PATCH] watchdog: Quiet down the boot messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1339123274.14838.35.camel@joe2Laptop>
Date:	Thu, 07 Jun 2012 19:41:14 -0700
From:	Joe Perches <joe@...ches.com>
To:	Don Zickus <dzickus@...hat.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>, nzimmer@....com
Subject: Re: [PATCH] watchdog: Quiet down the boot messages

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/