[PATCH 0/2] oprofile cleanups

[PATCH 0/2] oprofile cleanups

[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?

Thanks,

-Robert

[PATCH 1/2] oprofile, s390: Cleanups

[Robert Richter]

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 <robert.richter@amd.com>
---
 arch/Kconfig              |    3 ---
 arch/s390/Kconfig         |    1 -
 arch/s390/oprofile/init.c |    6 +++---
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 43abf3c..f78c2be 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -30,9 +30,6 @@ config OPROFILE_EVENT_MULTIPLEX
 config HAVE_OPROFILE
 	bool
 
-config HAVE_HWSAMPLER
-	bool
-
 config KPROBES
 	bool "Kprobes"
 	depends on MODULES
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 0cf20ad..ff19efd 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -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
diff --git a/arch/s390/oprofile/init.c b/arch/s390/oprofile/init.c
index 0e38a5b..16c76de 100644
--- a/arch/s390/oprofile/init.c
+++ b/arch/s390/oprofile/init.c
@@ -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;
 
-- 
1.7.3.4

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

[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 <robert.richter@amd.com>
---
 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(-)

diff --git a/arch/alpha/oprofile/common.c b/arch/alpha/oprofile/common.c
index bd8ac53..49b7246 100644
--- a/arch/alpha/oprofile/common.c
+++ b/arch/alpha/oprofile/common.c
@@ -183,7 +183,4 @@ oprofile_arch_init(struct oprofile_operations *ops)
 }
 
 
-void
-oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }
diff --git a/arch/avr32/oprofile/op_model_avr32.c b/arch/avr32/oprofile/op_model_avr32.c
index a3e9b3c..93feba4 100644
--- a/arch/avr32/oprofile/op_model_avr32.c
+++ b/arch/avr32/oprofile/op_model_avr32.c
@@ -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) { }
diff --git a/arch/blackfin/oprofile/bfin_oprofile.c b/arch/blackfin/oprofile/bfin_oprofile.c
index c3b9713..7dcac13 100644
--- a/arch/blackfin/oprofile/bfin_oprofile.c
+++ b/arch/blackfin/oprofile/bfin_oprofile.c
@@ -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) { }
diff --git a/arch/ia64/oprofile/init.c b/arch/ia64/oprofile/init.c
index 31b545c..a28feac 100644
--- a/arch/ia64/oprofile/init.c
+++ b/arch/ia64/oprofile/init.c
@@ -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();
diff --git a/arch/m32r/oprofile/init.c b/arch/m32r/oprofile/init.c
index fa56860..a7e063e 100644
--- a/arch/m32r/oprofile/init.c
+++ b/arch/m32r/oprofile/init.c
@@ -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) { }
diff --git a/arch/microblaze/oprofile/microblaze_oprofile.c b/arch/microblaze/oprofile/microblaze_oprofile.c
index def17e5..23c108d 100644
--- a/arch/microblaze/oprofile/microblaze_oprofile.c
+++ b/arch/microblaze/oprofile/microblaze_oprofile.c
@@ -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) { }
diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c
index f9eb1ab..fff011a 100644
--- a/arch/mips/oprofile/common.c
+++ b/arch/mips/oprofile/common.c
@@ -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();
diff --git a/arch/mn10300/oprofile/op_model_null.c b/arch/mn10300/oprofile/op_model_null.c
index cd4ab374..d308d13 100644
--- a/arch/mn10300/oprofile/op_model_null.c
+++ b/arch/mn10300/oprofile/op_model_null.c
@@ -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) { }
diff --git a/arch/parisc/oprofile/init.c b/arch/parisc/oprofile/init.c
index 026cba2..86cd3a5 100644
--- a/arch/parisc/oprofile/init.c
+++ b/arch/parisc/oprofile/init.c
@@ -18,6 +18,4 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 }
 
 
-void oprofile_arch_exit(void)
-{
-}
+void __exit oprofile_arch_exit(void) { }
diff --git a/arch/powerpc/oprofile/common.c b/arch/powerpc/oprofile/common.c
index d65e68f..c090951 100644
--- a/arch/powerpc/oprofile/common.c
+++ b/arch/powerpc/oprofile/common.c
@@ -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) { }
diff --git a/arch/s390/oprofile/init.c b/arch/s390/oprofile/init.c
index 16c76de..f4383ab 100644
--- a/arch/s390/oprofile/init.c
+++ b/arch/s390/oprofile/init.c
@@ -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();
 }
diff --git a/arch/sparc/oprofile/init.c b/arch/sparc/oprofile/init.c
index f9024bc..315c932 100644
--- a/arch/sparc/oprofile/init.c
+++ b/arch/sparc/oprofile/init.c
@@ -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) { }
diff --git a/arch/x86/oprofile/init.c b/arch/x86/oprofile/init.c
index cdfe4c5..085d952 100644
--- a/arch/x86/oprofile/init.c
+++ b/arch/x86/oprofile/init.c
@@ -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();
-- 
1.7.3.4

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

[Mike Frysinger]

On Wed, Mar 16, 2011 at 13:58, Robert Richter wrote:
>  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

Re: [PATCH 0/2] oprofile cleanups

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> 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

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

[Robert Richter]

On 16.03.11 16:15:49, Mike Frysinger wrote:
> On Wed, Mar 16, 2011 at 13:58, Robert Richter wrote:
> >  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

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

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

[Mike Frysinger]

On Thu, Mar 17, 2011 at 07:41, Robert Richter wrote:
> On 16.03.11 16:15:49, Mike Frysinger wrote:
>> On Wed, Mar 16, 2011 at 13:58, Robert Richter wrote:
>> >  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.
>
> 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.

> 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