From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S935335AbdJJUIo (ORCPT <rfc822;w@1wt.eu>);
        Tue, 10 Oct 2017 16:08:44 -0400
Received: from mga05.intel.com ([192.55.52.43]:46064 "EHLO mga05.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S935316AbdJJUIm (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 10 Oct 2017 16:08:42 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.43,359,1503385200"; 
   d="scan'208";a="1229312521"
Date: Tue, 10 Oct 2017 13:08:41 -0700
From: "Luck, Tony" <tony.luck@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Jeremy Cline <jcline@redhat.com>, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
        x86@kernel.org, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org, Laura Abbott <labbott@redhat.com>
Subject: Re: x86/mce: suspicious RCU usage in 4.13.4
Message-ID: <20171010200841.uhniuwc7pulqgmic@intel.com>
References: <9840bf0d-6756-75eb-1a2d-d3aace235244@redhat.com>
 <20171010194426.s7keveirclglx6vh@pd.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171010194426.s7keveirclglx6vh@pd.tnic>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>  	for (;;) {
>  		entry = mce_log_get_idx_check(mcelog.next);

Can't this get even simpler? Do we need the loop? The mutex
will now protect us while we check to see if there is a slot
to stash this new entry. Also just say:

		entry = mcelog.next;

>  		for (;;) {
> @@ -66,10 +67,10 @@ static int dev_mce_log(struct notifier_block *nb, unsigned long val,
>  			 * interesting ones:
>  			 */
>  			if (entry >= MCE_LOG_LEN) {
> -				set_bit(MCE_OVERFLOW,
> -					(unsigned long *)&mcelog.flags);
> +				set_bit(MCE_OVERFLOW, (unsigned long *)&mcelog.flags);

Need to mutex_unlock(&mce_chrdev_read_mutex); here.

>  				return NOTIFY_OK;
>  			}
> +
>  			/* Old left over entry. Skip: */
>  			if (mcelog.entry[entry].finished) {
>  				entry++;
> @@ -77,15 +78,13 @@ static int dev_mce_log(struct notifier_block *nb, unsigned long val,
>  			}
>  			break;
>  		}
> -		smp_rmb();
> -		next = entry + 1;
> -		if (cmpxchg(&mcelog.next, entry, next) == entry)
> -			break;

Ummm. Without this "break" how will we exit the loop (more fuel
for getting rid of the loop.

> +		mcelog.next = entry + 1;
>  	}

-Tony