xen/mce: don't issue error message for failed /dev/mcelog registration

PATCH Superseded Juergen Gross 3 participants 13 messages Jun 13, 2017 – Jun 14, 2017

Subsystems (2)

x86 Xen hypervisor interface

lore

Share & tools

lore

Juergen Gross

author maintainer

2017-06-13 13:45 UTC #0 lore raw

PATCH v2 1/1 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 <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
 drivers/xen/mcelog.c                    |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)

arch/x86/kernel/cpu/mcheck/dev-mcelog.c +9 -2

index 9c632cb88546..4eb5f7d1d593 100644
@@ -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;
 }

drivers/xen/mcelog.c +2 -0

index a493c7315e94..6cc1c15bcd84 100644
@@ -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:

Signature · 1 line

2.12.3

Ingo Molnar

2017-06-13 15:20 UTC #1 lore raw

Quoted 19 lines · Juergen Gross

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 <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
 drivers/xen/mcelog.c                    |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
index 9c632cb88546..4eb5f7d1d593 100644
@@ -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) {

arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398

Quoted 10 lines

arch/x86/kernel/cpu/mcheck/dev-mcelog.c +8 -2

-		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

Juergen Gross

author maintainer

2017-06-13 16:13 UTC #2 lore raw

Quoted 26 lines · Ingo Molnar

Quoted 23 lines · Juergen Gross
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 <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
 drivers/xen/mcelog.c                    |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
index 9c632cb88546..4eb5f7d1d593 100644
@@ -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. */

Quoted 8 lines

arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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

Joe Perches

2017-06-13 16:32 UTC #3 lore raw

Quoted 7 lines · Juergen Gross

Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.

[]

Quoted 1 line

Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0

[]

Quoted 17 lines

Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.

Quoted 1 line

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

Juergen Gross

author maintainer

2017-06-13 19:27 UTC #4 lore raw

Quoted 30 lines · Joe Perches

Quoted 7 lines · Juergen Gross
Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.
[]
Quoted 1 line
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
[]
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.
Quoted 1 line
Okay, will change.

Quoted 7 lines

Perhaps better is to reverse the test

		if (err != -EBUSY) {
			pr_err("Unable to ....", err);
			return err;
		}
		pr_info("etc...");

Okay.

Quoted 16 lines

[ 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

Quoted 8 lines

	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

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Juergen Gross

author maintainer

2017-06-13 19:27 UTC #5 lore raw

In reply to Joe Perches

Quoted 30 lines · Joe Perches

Quoted 7 lines · Juergen Gross
Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.
[]
Quoted 1 line
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
[]
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.
Quoted 1 line
Okay, will change.

Quoted 7 lines

Perhaps better is to reverse the test

		if (err != -EBUSY) {
			pr_err("Unable to ....", err);
			return err;
		}
		pr_info("etc...");

Okay.

Quoted 16 lines

[ 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

Quoted 8 lines

	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

Joe Perches

2017-06-13 19:30 UTC #6 lore raw

Quoted 68 lines · Juergen Gross

Quoted 32 lines · Joe Perches
Quoted 7 lines · Juergen Gross
Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.
[]
Quoted 1 line
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
[]
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.
Quoted 1 line
Okay, will change.
Quoted 7 lines
Perhaps better is to reverse the test
		if (err != -EBUSY) {
			pr_err("Unable to ....", err);
			return err;
		}
		pr_info("etc...");
Okay.
Quoted 16 lines
[ 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
Quoted 8 lines
	err = misc_register(&mce_chrdev_device);
	if (err == -EBUSY) {
		pr_info(...);
		return err;
	} else if (err) {
		pr_err(...);
		return err;
	}

Quoted 2 lines

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.

Juergen Gross

author maintainer

2017-06-14 07:49 UTC #7 lore raw

Quoted 69 lines · Joe Perches

Quoted 68 lines · Juergen Gross
Quoted 32 lines · Joe Perches
Quoted 7 lines · Juergen Gross
Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.
[]
Quoted 1 line
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
[]
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.
Quoted 1 line
Okay, will change.
Quoted 7 lines
Perhaps better is to reverse the test
		if (err != -EBUSY) {
			pr_err("Unable to ....", err);
			return err;
		}
		pr_info("etc...");
Okay.
Quoted 24 lines
[ 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;
	}

Quoted 5 lines

Quoted 2 lines
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

Juergen Gross

author maintainer

2017-06-14 07:49 UTC #8 lore raw

In reply to Joe Perches

Quoted 69 lines · Joe Perches

Quoted 68 lines · Juergen Gross
Quoted 32 lines · Joe Perches
Quoted 7 lines · Juergen Gross
Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.
[]
Quoted 1 line
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
[]
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.
Quoted 1 line
Okay, will change.
Quoted 7 lines
Perhaps better is to reverse the test
		if (err != -EBUSY) {
			pr_err("Unable to ....", err);
			return err;
		}
		pr_info("etc...");
Okay.
Quoted 24 lines
[ 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;
	}

Quoted 5 lines

Quoted 2 lines
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

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Joe Perches

2017-06-13 19:30 UTC #9 lore raw

In reply to Juergen Gross

Quoted 68 lines · Juergen Gross

Quoted 32 lines · Joe Perches
Quoted 7 lines · Juergen Gross
Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.
[]
Quoted 1 line
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
[]
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.
Quoted 1 line
Okay, will change.
Quoted 7 lines
Perhaps better is to reverse the test
		if (err != -EBUSY) {
			pr_err("Unable to ....", err);
			return err;
		}
		pr_info("etc...");
Okay.
Quoted 16 lines
[ 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
Quoted 8 lines
	err = misc_register(&mce_chrdev_device);
	if (err == -EBUSY) {
		pr_info(...);
		return err;
	} else if (err) {
		pr_err(...);
		return err;
	}

Quoted 2 lines

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.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Joe Perches

2017-06-13 16:32 UTC #10 lore raw

In reply to Juergen Gross

Quoted 7 lines · Juergen Gross

Quoted 6 lines · Ingo Molnar
Quoted 4 lines · Juergen Gross
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.

[]

Quoted 1 line

Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c
Quoted 1 line
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0

[]

Quoted 17 lines

Quoted 8 lines
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
@@ -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. */
Quoted 8 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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.

Quoted 1 line

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

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Juergen Gross

author maintainer

2017-06-13 16:13 UTC #11 lore raw

In reply to Ingo Molnar

Quoted 26 lines · Ingo Molnar

Quoted 23 lines · Juergen Gross
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 <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
 drivers/xen/mcelog.c                    |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +2 -2
index 9c632cb88546..4eb5f7d1d593 100644
@@ -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. */

Quoted 8 lines

arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +6 -0
+			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

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Ingo Molnar

2017-06-13 15:20 UTC #12 lore raw

In reply to Juergen Gross

Quoted 19 lines · Juergen Gross

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 <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 11 +++++++++--
 drivers/xen/mcelog.c                    |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/mcheck/dev-mcelog.c +0 -0
index 9c632cb88546..4eb5f7d1d593 100644
@@ -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) {

arch/x86/kernel/cpu/mcheck/dev-mcelog.c static __init int dev_mcelog_init_device(void) · line 398

Quoted 10 lines

arch/x86/kernel/cpu/mcheck/dev-mcelog.c +8 -2

-		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

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel