oprofile cleanups

PATCH In review Robert Richter 3 participants 7 messages Mar 16, 2011 – Mar 17, 2011

Subsystems (8)

x86 PowerPC MIPS s390 +4 more subsystems

lore

Share & tools

lore

Robert Richter

author

2011-03-16 18:06 UTC #0 lore raw

cover oprofile cleanups

These 2 patches contain some cleanups I made while reviewing oprofile s390 code. I already applied them to my oprofile git tree, but please take a look at it anyway.

Ingo, since the patches are cleanups, I think it is ok to add them to my oprofile pull request for .39 for you that I will send out soon. All other patches are already in linux-next for several weeks. Is this ok for you too?

Thanks,

-Robert

Robert Richter

author

2011-03-16 18:05 UTC #1 lore raw

PATCH 1/2 oprofile, s390: Cleanups

Remove unused HAVE_HWSAMPLER config option. It is not used anymore, removing it.

Also make some functions static and some coding style fixes.

Signed-off-by Robert Richter <[email protected]>

---
 arch/Kconfig              |    3 ---
 arch/s390/Kconfig         |    1 -
 arch/s390/oprofile/init.c |    6 +++---
 3 files changed, 3 insertions(+), 7 deletions(-)

arch/Kconfig +0 -3

index 43abf3c..f78c2be 100644
@@ -30,9 +30,6 @@ config OPROFILE_EVENT_MULTIPLEX
 config HAVE_OPROFILE
 	bool
 
-config HAVE_HWSAMPLER
-	bool
-
 config KPROBES
 	bool "Kprobes"
 	depends on MODULES

arch/s390/Kconfig +0 -1

index 0cf20ad..ff19efd 100644
@@ -115,7 +115,6 @@ config S390
 	select ARCH_INLINE_WRITE_UNLOCK_BH
 	select ARCH_INLINE_WRITE_UNLOCK_IRQ
 	select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
-	select HAVE_HWSAMPLER
 
 config SCHED_OMIT_FRAME_POINTER
 	def_bool y

arch/s390/oprofile/init.c +3 -3

index 0e38a5b..16c76de 100644
@@ -133,7 +133,7 @@ static int oprofile_create_hwsampling_files(struct super_block *sb,
 	return 0;
 }
 
-int oprofile_hwsampler_init(struct oprofile_operations* ops)
+static int oprofile_hwsampler_init(struct oprofile_operations *ops)
 {
 	if (hwsampler_setup())
 		return -ENODEV;

@@ -166,13 +166,13 @@ int oprofile_hwsampler_init(struct oprofile_operations* ops)
 	return 0;
 }
 
-void oprofile_hwsampler_exit(void)
+static void oprofile_hwsampler_exit(void)
 {
 	oprofile_timer_exit();
 	hwsampler_shutdown();
 }
 
-int __init oprofile_arch_init(struct oprofile_operations* ops)
+int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
 	ops->backtrace = s390_backtrace;

Signature · 1 line

1.7.3.4

Robert Richter

author

2011-03-16 18:05 UTC #2 lore raw

PATCH 2/2 oprofile: Add __exit attibute to oprofile_arch_exit() functions

In reply to Robert Richter

After

979048e oprofile: don't call arch exit code from init code on failure

we may add __exit attibutes to oprofile_arch_exit() functions.

Signed-off-by Robert Richter <[email protected]>

---
 arch/alpha/oprofile/common.c                   |    5 +----
 arch/avr32/oprofile/op_model_avr32.c           |    5 +----
 arch/blackfin/oprofile/bfin_oprofile.c         |    4 +---
 arch/ia64/oprofile/init.c                      |    2 +-
 arch/m32r/oprofile/init.c                      |    4 +---
 arch/microblaze/oprofile/microblaze_oprofile.c |    4 +---
 arch/mips/oprofile/common.c                    |    2 +-
 arch/mn10300/oprofile/op_model_null.c          |    5 +----
 arch/parisc/oprofile/init.c                    |    4 +---
 arch/powerpc/oprofile/common.c                 |    4 +---
 arch/s390/oprofile/init.c                      |    2 +-
 arch/sparc/oprofile/init.c                     |    4 +---
 arch/x86/oprofile/init.c                       |    2 +-
 13 files changed, 13 insertions(+), 34 deletions(-)

arch/alpha/oprofile/common.c +1 -4

index bd8ac53..49b7246 100644
@@ -183,7 +183,4 @@ oprofile_arch_init(struct oprofile_operations *ops)
 }
 
 
-void
-oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }

arch/avr32/oprofile/op_model_avr32.c +1 -4

index a3e9b3c..93feba4 100644
@@ -232,7 +232,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return 0;
 }
 
-void oprofile_arch_exit(void)
-{
-
-}
+void __exit oprofile_arch_exit(void) { }

arch/blackfin/oprofile/bfin_oprofile.c +1 -3

index c3b9713..7dcac13 100644
@@ -13,6 +13,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return -1;
 }
 
-void oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }

arch/ia64/oprofile/init.c +1 -1

index 31b545c..a28feac 100644
@@ -30,7 +30,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 }
 
 
-void oprofile_arch_exit(void)
+void __exit oprofile_arch_exit(void)
 {
 #ifdef CONFIG_PERFMON
 	perfmon_exit();

arch/m32r/oprofile/init.c +1 -3

index fa56860..a7e063e 100644
@@ -17,6 +17,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return -ENODEV;
 }
 
-void oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }

arch/microblaze/oprofile/microblaze_oprofile.c +1 -3

index def17e5..23c108d 100644
@@ -17,6 +17,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return -1;
 }
 
-void oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }

arch/mips/oprofile/common.c +1 -1

index f9eb1ab..fff011a 100644
@@ -122,7 +122,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return 0;
 }
 
-void oprofile_arch_exit(void)
+void __exit oprofile_arch_exit(void)
 {
 	if (model)
 		model->exit();

arch/mn10300/oprofile/op_model_null.c +1 -4

index cd4ab374..d308d13 100644
@@ -16,7 +16,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return -ENODEV;
 }
 
-void oprofile_arch_exit(void)
-{
-}
-
+void __exit oprofile_arch_exit(void) { }

arch/parisc/oprofile/init.c +1 -3

index 026cba2..86cd3a5 100644
@@ -18,6 +18,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 }
 
 
-void oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }

arch/powerpc/oprofile/common.c +1 -3

index d65e68f..c090951 100644
@@ -249,6 +249,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return 0;
 }
 
-void oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }

arch/s390/oprofile/init.c +1 -1

index 16c76de..f4383ab 100644
@@ -179,7 +179,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return oprofile_hwsampler_init(ops);
 }
 
-void oprofile_arch_exit(void)
+void __exit oprofile_arch_exit(void)
 {
 	oprofile_hwsampler_exit();
 }

arch/sparc/oprofile/init.c +1 -3

index f9024bc..315c932 100644
@@ -82,6 +82,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return ret;
 }
 
-void oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }

arch/x86/oprofile/init.c +1 -1

index cdfe4c5..085d952 100644
@@ -41,7 +41,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 }
 
 
-void oprofile_arch_exit(void)
+void __exit oprofile_arch_exit(void)
 {
 #ifdef CONFIG_X86_LOCAL_APIC
 	op_nmi_exit();

Signature · 1 line

1.7.3.4

Mike Frysinger

2011-03-16 20:16 UTC #3 lore raw

Re: [PATCH 2/2] oprofile: Add __exit attibute to oprofile_arch_exit() functions

Quoted 3 lines · Robert Richter

979048e oprofile: don't call arch exit code from init code on failure
we may add __exit attibutes to oprofile_arch_exit() functions.

i dont think this the way to go. how about updating one place (include/linux/oprofile.h:oprofile_arch_exit) and making sure all arch files are including that header if they arent already ? after all, if they arent including that header, the arch code could break without noticing.
-mike

Robert Richter

author

2011-03-17 11:41 UTC #4 lore raw

Re: [PATCH 2/2] oprofile: Add __exit attibute to oprofile_arch_exit() functions

Quoted 10 lines · Mike Frysinger

Quoted 3 lines · Robert Richter
979048e oprofile: don't call arch exit code from init code on failure
we may add __exit attibutes to oprofile_arch_exit() functions.
i dont think this the way to go. how about updating one place (include/linux/oprofile.h:oprofile_arch_exit) and making sure all arch files are including that header if they arent already ? after all, if they arent including that header, the arch code could break without noticing.

Mike,

do you mean we specify the __exit attribute in the function declaration of the header file and make sure it is included everythere? I was looking at current implementations in the kernel and this is not common. Mostly the attributes are set in the function definition. So I was not sure if that would work. If so, may we skip then the __exit attribute in the definition?

GCC doc states: "The keyword __attribute__ allows you to specify special attributes when making a declaration."

http://gcc.gnu.org/onlinedocs/gcc-4.5.1/gcc/Function-Attributes.html#Function%20Attributes

but nothing about that happens if it is in the function definition and this is different from the declaration.

-Robert

Signature · 2 lines

Advanced Micro Devices, Inc.
Operating System Research Center

Mike Frysinger

2011-03-17 19:16 UTC #5 lore raw

Re: [PATCH 2/2] oprofile: Add __exit attibute to oprofile_arch_exit() functions

Quoted 12 lines · Robert Richter

Quoted 10 lines · Mike Frysinger
Quoted 3 lines · Robert Richter
979048e oprofile: don't call arch exit code from init code on failure
we may add __exit attibutes to oprofile_arch_exit() functions.
i dont think this the way to go. how about updating one place (include/linux/oprofile.h:oprofile_arch_exit) and making sure all arch files are including that header if they arent already ? after all, if they arent including that header, the arch code could break without noticing.

Quoted 5 lines

do you mean we specify the __exit attribute in the function declaration of the header file and make sure it is included everythere? I was looking at current implementations in the kernel and this is not common. Mostly the attributes are set in the function definition.

i think that's because for most functions, there is no matching prototype in common code. drivers themselves have no reason to create such prototypes.

Quoted 10 lines

So I was not sure if that would work. If so, may we skip then the __exit attribute in the definition?
GCC doc states: "The keyword __attribute__ allows you to specify special attributes when making a declaration."
http://gcc.gnu.org/onlinedocs/gcc-4.5.1/gcc/Function-Attributes.html#Function%20Attributes
but nothing about that happens if it is in the function definition and this is different from the declaration.

if the function prototype is seen in the same file as the function definition, attributes are carried over. if it isnt (because the oprofile header isnt being included), then the attribute will be ignored for the function definition.
-mike

Ingo Molnar

2011-03-17 08:09 UTC #6 lore raw

In reply to Robert Richter

Quoted 8 lines · Robert Richter

These 2 patches contain some cleanups I made while reviewing oprofile s390 code. I already applied them to my oprofile git tree, but please take a look at it anyway.
Ingo, since the patches are cleanups, I think it is ok to add them to my oprofile pull request for .39 for you that I will send out soon. All other patches are already in linux-next for several weeks. Is this ok for you too?

Yeah, assuming Mike's remarks have been addressed.

Thanks,

Ingo