From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754470AbdFNHtR (ORCPT ); Wed, 14 Jun 2017 03:49:17 -0400 Received: from mx2.suse.de ([195.135.220.15]:33238 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750751AbdFNHtQ (ORCPT ); Wed, 14 Jun 2017 03:49:16 -0400 Subject: Re: [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration To: Joe Perches , Ingo Molnar Cc: linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, x86@kernel.org, boris.ostrovsky@oracle.com, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com, tony.luck@intel.com, bp@alien8.de References: <20170613134507.16983-1-jgross@suse.com> <20170613152014.tax42aevmsfdopxa@gmail.com> <498f5ad7-c203-9b21-c153-e3496755b88d@suse.com> <1497371514.18751.17.camel@perches.com> <3a7376bd-1a93-1bc2-f7b6-f2a95f641a5d@suse.com> <1497382223.18751.23.camel@perches.com> From: Juergen Gross Message-ID: <762259f8-c0c8-c476-c5b6-c41801f9192e@suse.com> Date: Wed, 14 Jun 2017 09:49:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1497382223.18751.23.camel@perches.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13/06/17 21:30, Joe Perches wrote: > On Tue, 2017-06-13 at 21:27 +0200, Juergen Gross wrote: >> On 13/06/17 18:31, Joe Perches wrote: >>> On Tue, 2017-06-13 at 18:13 +0200, Juergen Gross wrote: >>>> On 13/06/17 17:20, Ingo Molnar wrote: >>>>> * Juergen Gross wrote: >>>>> >>>>>> When running under Xen as dom0 /dev/mcelog is being registered by Xen >>>>>> instead of the normal mcelog driver. Avoid an error message being >>>>>> issued by the mcelog driver in this case. Instead issue an informative >>>>>> message that Xen has registered the device. >>> >>> [] >>>>>> diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c >>> >>> [] >>>>>> @@ -388,9 +388,16 @@ static __init int dev_mcelog_init_device(void) >>>>>> /* register character device /dev/mcelog */ >>>>>> err = misc_register(&mce_chrdev_device); >>>>>> if (err) { >>>>>> - pr_err("Unable to init device /dev/mcelog (rc: %d)\n", err); >>>>>> - return err; >>>>>> + if (err == -EBUSY) >>>>>> + /* Xen dom0 might have registered the device already. */ >>>>>> + pr_info("Unable to init device /dev/mcelog, already registered"); >>>>>> + else { >>>>>> + pr_err("Unable to init device /dev/mcelog (rc: %d)\n", >>>>>> + err); >>>>>> + return err; >>>>>> + } >>>>> >>>>> Please only use balanced curly braces in conditional statements. >>>> >>>> Okay, will change. >>> >>> Perhaps better is to reverse the test >>> >>> if (err != -EBUSY) { >>> pr_err("Unable to ....", err); >>> return err; >>> } >>> pr_info("etc..."); >> >> Okay. >> >>> >>> [ rest of code at this indentation ] >>> >>> but it looks like you added a logic defect too and >>> this code should be: >>> >>> if (err) { >>> if (err == -EBUSY) >>> pr_info(...) >>> else >>> pr_err(...) >>> return err; >>> } >>> >>> or less indented using >>> >>> err = misc_register(&mce_chrdev_device); >>> if (err == -EBUSY) { >>> pr_info(...); >>> return err; >>> } else if (err) { >>> pr_err(...); >>> return err; >>> } >> >> I didn't want to omit the call to mce_register_decode_chain() in the Xen >> case. > > You should definitely mention that behavior change > in the changelog. Looking more thoroughly it seems I really don't need the call for Xen. So thanks for bringing it up. :-) Juergen