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 S935230AbdJJUOB (ORCPT <rfc822;w@1wt.eu>);
        Tue, 10 Oct 2017 16:14:01 -0400
Received: from mga06.intel.com ([134.134.136.31]:53569 "EHLO mga06.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S933881AbdJJUN7 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 10 Oct 2017 16:13:59 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.43,359,1503385200"; 
   d="scan'208";a="1180653689"
From: Andi Kleen <ak@linux.intel.com>
To: "Luck\, Tony" <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>, 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
References: <9840bf0d-6756-75eb-1a2d-d3aace235244@redhat.com>
        <20171010194426.s7keveirclglx6vh@pd.tnic>
        <20171010200841.uhniuwc7pulqgmic@intel.com>
Date: Tue, 10 Oct 2017 13:13:58 -0700
In-Reply-To: <20171010200841.uhniuwc7pulqgmic@intel.com> (Tony Luck's message
        of "Tue, 10 Oct 2017 13:08:41 -0700")
Message-ID: <871smasqux.fsf@linux.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

"Luck, Tony" <tony.luck@intel.com> writes:

>>  	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:

IMHO the warning is just bogus. There's nothing here that actually
uses RCU. I would just remove it.

>>  			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.

And yes that too.

-Andi