x86, mce: cleanup initcall of debugfs for mce
PATCH 1/1 x86, mce: cleanup initcall of debugfs for mce
(Send this patch again because I posted the patch as an attachment.
This patch is made based on the v5 patches of Huangying for mce. Before using this patch, please apply the patches of Huangying. The patches' URLs of Huangying are listed as following:
1.x86: MCE: Support specifying context for software mce injection
URL: http://marc.info/?l=linux-kernel&m=124900459100786&w=2
2.x86: MCE: Support specifying raise mode for software MCE injection
URL: http://marc.info/?l=linux-kernel&m=124900459100795&w=2
3.x86: MCE: Move debugfs mce dir creating to mce.c
URL: http://marc.info/?l=linux-kernel&m=124900459100790&w=2
4.x86: MCE: Fake panic support for MCE testing
URL: http://marc.info/?l=linux-kernel&m=124900453800724&w=2
There are two initcall functions in the patches of Huangying, mce_debugfs_init() used in mce.c and severities_debugfs_init() used in mce-severity.c. I think an initcall function is enough for mce debug. So we make them into one initcall function.
This version is based x86-tip/mce.
--- arch/x86/kernel/cpu/mcheck/mce-internal.h | 2 +- arch/x86/kernel/cpu/mcheck/mce-severity.c | 13 +++---------- arch/x86/kernel/cpu/mcheck/mce.c | 24 +++++++++++------------- 3 files changed, 15 insertions(+), 24 deletions(-)
(quoted diff) +0 -0
b/arch/x86/kernel/cpu/mcheck/mce-internal.h
index 32996f9..ca8421d 100644
arch/x86/kernel/cpu/mcheck/mce-internal.h +1 -1
@@ -22,7 +22,7 @@ struct mce_bank { };int mce_severity (struct mce *a,int tolerant,char **msg); -struct dentry *mce_get_debugfs_dir (void ); +int __init severities_debugfs_init (struct dentry *dmce);extern int mce_ser;
(quoted diff) +0 -0
b/arch/x86/kernel/cpu/mcheck/mce-severity.c
index bc35a07..49e57ab 100644
arch/x86/kernel/cpu/mcheck/mce-severity.c +0 -0
@@ -193,22 +193,15 @@ static const struct file_operations
severities_coverage_fops = {
.write = severities_coverage_write,
};
-static int __init severities_debugfs_init(void) +int __init severities_debugfs_init(struct dentry *dmce)
{
- struct dentry *dmce = NULL, *fseverities_coverage = NULL;
+ struct dentry *fseverities_coverage = NULL;
- dmce = mce_get_debugfs_dir();
- if (dmce == NULL)
- goto err_out;
fseverities_coverage = debugfs_create_file("severities-coverage",
0444, dmce, NULL, &severities_coverage_fops);
if (fseverities_coverage == NULL)
- goto err_out;
+ return -ENOMEM;
return 0;
-
-err_out:
- return -ENOMEM;
}
-late_initcall(severities_debugfs_init);
arch/x86/kernel/cpu/mcheck/mce.c +11 -13
@@ -2018,16 +2018,6 @@ static int __init mcheck_disable(char *str)__setup ("nomce" , mcheck_disable);#ifdef CONFIG_DEBUG_FS -struct dentry *mce_get_debugfs_dir (void ) -{ -static struct dentry *dmce; - -if (!dmce) - dmce =debugfs_create_dir ("mce" ,NULL ); - -return dmce; -} -static void mce_reset (void ) { cpu_missing =0 ;
@@ -2055,17 +2045,25 @@ DEFINE_SIMPLE_ATTRIBUTE(fake_panic_fops, fake_panic_get,static int __init mce_debugfs_init (void ) { -struct dentry *dmce, *ffake_panic; +struct dentry *dmce =NULL , *ffake_panic =NULL ; - dmce =mce_get_debugfs_dir (); + dmce =debugfs_create_dir ("mce" ,NULL );if (!dmce)return -ENOMEM ; ffake_panic =debugfs_create_file ("fake_panic" ,0444 , dmce,NULL , &fake_panic_fops);if (!ffake_panic) -return -ENOMEM ; + goto err_out; + +if (severities_debugfs_init (dmce)) + goto err_out;return 0 ; + +err_out: +debugfs_remove_recursive (dmce); + +return -ENOMEM ; }late_initcall (mce_debugfs_init);#endif
Signature · 1 line
1.6.2.2
arch/x86/kernel/cpu/mcheck/mce-internal.h struct mce_bank { · line 25
Quoted 4 lines
int
mce_severity(struct mce *a, int tolerant, char **msg);arch/x86/kernel/cpu/mcheck/mce-internal.h +1
-1-struct dentry *mce_get_debugfs_dir (void ); +int __init severities_debugfs_init (struct dentry *dmce);
__init is not needed on prototypes.
Sorry but I think it's better to have an own initcall per file, that makes everything more self contained and easier to read.
-Andi
Jin Dongming
Reviewed-by Hidetoshi SetoQuoted 6 lines · Andi Kleen
Sorry but I think it's better to have an own initcall per file, that makes everything more self contained and easier to read.
-Andi
Thank you for your comment.
That's OK, but this patch is also used for resolving compile error without CONFIG_DEBUG_FS. Then how about this patch?
Best regards,
Jin Dongming
===
[PATCH] x86,mce: Add ifdef for severities_debugfs_init()
This patch is made based on the v5 patches of Huangying for mce. Before using this patch, please apply the patches of Huangying. The patches' URLs of Huangying are listed as following:
1.x86: MCE: Support specifying context for software mce injection
URL: http://marc.info/?l=linux-kernel&m=124900459100786&w=2
2.x86: MCE: Support specifying raise mode for software MCE injection
URL: http://marc.info/?l=linux-kernel&m=124900459100795&w=2
3.x86: MCE: Move debugfs mce dir creating to mce.c
URL: http://marc.info/?l=linux-kernel&m=124900459100790&w=2
4.x86: MCE: Fake panic support for MCE testing
URL: http://marc.info/?l=linux-kernel&m=124900453800724&w=2
When the kernel is compiled without CONFIG_DEBUG_FS, compiler could not find out mce_get_debugfs_dir() which is used by severities_debugfs_init() and output the error message.
This version is based x86-tip/mce.
--- arch/x86/kernel/cpu/mcheck/mce-internal.h | 3 +++ arch/x86/kernel/cpu/mcheck/mce-severity.c | 2 ++ 2 files changed, 5 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/mcheck/mce-internal.h +3 -0
@@ -22,7 +22,10 @@ struct mce_bank { };int mce_severity (struct mce *a,int tolerant,char **msg); + +#ifdef CONFIG_DEBUG_FS struct dentry *mce_get_debugfs_dir (void ); +#endif extern int mce_ser;
arch/x86/kernel/cpu/mcheck/mce-severity.c +2 -0
@@ -139,6 +139,7 @@ int mce_severity(struct mce *a, int tolerant, char **msg) } } +#ifdef CONFIG_DEBUG_FS static void *s_start (struct seq_file *f,loff_t *pos) {if (*pos >=ARRAY_SIZE (severities))
@@ -212,3 +213,4 @@ err_out: return -ENOMEM; }late_initcall (severities_debugfs_init); +#endif
Signature · 1 line
1.6.2.2
Quoted 10 lines · Jin Dongming
Quoted 5 lines · Andi Kleen
Sorry but I think it's better to have an own initcall per file, that makes everything more self contained and easier to read.
-Andi
Thank you for your comment.
That's OK, but this patch is also used for resolving compile error without
CONFIG_DEBUG_FS. Then how about this patch?
Compiler errors should be always fixed of course. But did you check the compile error is still in the latest version of the x86/mce branch ofgit://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git
It might be fixed already.
-Andi
Quoted 13 lines · Andi Kleen
Quoted 10 lines · Jin Dongming
Quoted 5 lines · Andi Kleen
Sorry but I think it's better to have an own initcall per file, that makes everything more self contained and easier to read.
-Andi
Thank you for your comment.
That's OK, but this patch is also used for resolving compile error without
CONFIG_DEBUG_FS. Then how about this patch?
Quoted 5 lines
Compiler errors should be always fixed of course. But did you check the compile error is still in the latest version of the x86/mce branch of
git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git
It might be fixed already.
The compiler error pointed by Jin will be introduced by Huang's patch:
3.x86: MCE: Move debugfs mce dir creating to mce.c
URL: http://marc.info/?l=linux-kernel&m=124900459100790&w=2
and it seems that none of tip branches have Huang's debug patch set yet. At least I could not find them in today's tip/x86/mce and tip/master. (Or I might have trouble on syncing of git.)
I think that the previous change to unify initcalls is worth to be in an incremental patch, but this trivial change to add ifdef could be done by updating original patch if you'd like to do so.
Thanks,
H.Seto