Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1664870 > unrolled thread

[PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration Juergen Gross <jgross@suse.com> - 2017-06-13 15:50 +0200
    Re: [PATCH v2] xen/mce: don't issue error message for failed  /dev/mcelog registration Ingo Molnar <mingo@kernel.org> - 2017-06-13 17:30 +0200
      Re: [PATCH v2] xen/mce: don't issue error message for failed  /dev/mcelog registration Juergen Gross <jgross@suse.com> - 2017-06-13 18:20 +0200
        Re: [PATCH v2] xen/mce: don't issue error message for failed  /dev/mcelog registration Joe Perches <joe@perches.com> - 2017-06-13 18:40 +0200
          Re: [PATCH v2] xen/mce: don't issue error message for failed  /dev/mcelog registration Juergen Gross <jgross@suse.com> - 2017-06-13 21:30 +0200
            Re: [PATCH v2] xen/mce: don't issue error message for failed  /dev/mcelog registration Joe Perches <joe@perches.com> - 2017-06-13 21:40 +0200
              Re: [PATCH v2] xen/mce: don't issue error message for failed  /dev/mcelog registration Juergen Gross <jgross@suse.com> - 2017-06-14 09:50 +0200

#1664870 — [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

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.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
 drivers/xen/mcelog.c                    |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
index 9c632cb88546..4eb5f7d1d593 100644
--- 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;
+		}
 	}
+
 	mce_register_decode_chain(&dev_mcelog_nb);
 	return 0;
 }
diff --git a/drivers/xen/mcelog.c b/drivers/xen/mcelog.c
index a493c7315e94..6cc1c15bcd84 100644
--- a/drivers/xen/mcelog.c
+++ b/drivers/xen/mcelog.c
@@ -408,6 +408,8 @@ static int __init xen_late_init_mcelog(void)
 	if (ret)
 		goto deregister;
 
+	pr_info("/dev/mcelog registered by Xen\n");
+
 	return 0;
 
 deregister:
-- 
2.12.3

[toc] | [next] | [standalone]

#1664959 — Re: [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

* Juergen Gross <jgross@suse.com> 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.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
>  drivers/xen/mcelog.c                    |  2 ++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
> index 9c632cb88546..4eb5f7d1d593 100644
> --- 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.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1664990 — Re: [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

On 13/06/17 17:20, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> 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.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
>>  drivers/xen/mcelog.c                    |  2 ++
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
>> index 9c632cb88546..4eb5f7d1d593 100644
>> --- 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.


Juergen

[toc] | [prev] | [next] | [standalone]

#1665010 — Re: [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

On Tue, 2017-06-13 at 18:13 +0200, Juergen Gross wrote:
> On 13/06/17 17:20, Ingo Molnar wrote:
> > * Juergen Gross <jgross@suse.com> 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...");

		[ 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;
	}

[toc] | [prev] | [next] | [standalone]

#1665123 — Re: [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

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 <jgross@suse.com> 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.


Juergen

[toc] | [prev] | [next] | [standalone]

#1665131 — Re: [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

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 <jgross@suse.com> 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.

[toc] | [prev] | [next] | [standalone]

#1665526 — Re: [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration

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 <jgross@suse.com> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web