Re: [PATCH] [20/28] x86: MCE: Switch x86 machine check handler to Monarch election.

[Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>]

Andi Kleen wrote:
> +/*
> + * Check if a timeout waiting for other CPUs happened.
> + */
> +static int mce_timed_out(u64 *t)
> +{
> +	/*
> +	 * The others already did panic for some reason.
> +	 * Bail out like in a timeout.
> +	 * rmb() to tell the compiler that system_state
> +	 * might have been modified by someone else.
> +	 */
> +	rmb();
> +	if (atomic_read(&mce_paniced))
> +		wait_for_panic();
> +	if (!monarch_timeout)
> +		goto out;
> +	if ((s64)*t < SPINUNIT) {
> +		/* CHECKME: Make panic default for 1 too? */
> +		if (tolerant < 1)
> +			mce_panic("Timeout synchronizing machine check over CPUs",
> +				  NULL, NULL);

Assuming that if we came here from mce_start() and panic, then I suppose no mce
log would be appeared on the console since no cpu have invoked mce_log(&m) yet.
Is it expected behavior?

> +		cpu_missing = 1;
> +		return 1;
> +	}
> +	*t -= SPINUNIT;
> +out:
> +	touch_nmi_watchdog();
> +	return 0;
> +}
(snip)
> +/*
> + * Start of Monarch synchronization. This waits until all CPUs have
> + * entered the ecception handler and then determines if any of them
                  ^^^^^^^^^
                  exception

> + * saw a fatal event that requires panic. Then it executes them
> + * in the entry order.
> + * TBD double check parallel CPU hotunplug
> + */
> +static int mce_start(int no_way_out, int *order)
> +{
> +	int nwo;
> +	int cpus = num_online_cpus();
> +	static atomic_t global_nwo;
> +	u64 timeout = (u64)monarch_timeout * NSEC_PER_USEC;
> +
> +	if (!timeout) {
> +		*order = -1;
> +		return no_way_out;
> +	}
> +
> +	atomic_add(no_way_out, &global_nwo);
> +
> +	/*
> +	 * Wait for everyone.
> +	 */
> +	while (atomic_read(&mce_callin) != cpus) {
> +		if (mce_timed_out(&timeout)) {
> +			atomic_set(&global_nwo, 0);
> +			*order = -1;
> +			return no_way_out;
> +		}
> +		ndelay(SPINUNIT);
> +	}
> +
> +	/*
> +	 * Cache the global no_way_out state.
> +	 */
> +	nwo = atomic_read(&global_nwo);
> +
> +	/*
> +	 * Monarch starts executing now, the others wait.
> +	 */
> +	if (*order == 1) {
> +		atomic_set(&global_nwo, 0);

Monarch should clear global_nwo after all Subjects have read it.
Or it should be cleared by last Subject instead.

> +		atomic_set(&mce_executing, 1);
> +		return nwo;
> +	}
> +
> +	/*
> +	 * Now start the scanning loop one by one
> +	 * in the original callin order.
> +	 * This way when there are any shared banks it will
> +	 * be only seen by one CPU before cleared, avoiding duplicates.
> +	 */
> +	while (atomic_read(&mce_executing) < *order) {
> +		if (mce_timed_out(&timeout)) {
> +			atomic_set(&global_nwo, 0);
> +			*order = -1;
> +			return no_way_out;
> +		}
> +		ndelay(SPINUNIT);
> +	}
> +	return nwo;
> +}

Thanks,
H.Seto