[PATCH -tip] x86: oprofile/op_model_amd.c set return values for op_amd_handle_ibs()

[PATCH -tip] x86: oprofile/op_model_amd.c set return values for op_amd_handle_ibs()

[Jaswinder Singh Rajput]

op_amd_handle_ibs() should return 0 when IBS is not present or not defined.

Fix compilation warning:
 CC [M]  arch/x86/oprofile/op_model_amd.o
 arch/x86/oprofile/op_model_amd.c: In function ‘op_amd_handle_ibs’:
 arch/x86/oprofile/op_model_amd.c:217: warning: no return statement in function returning non-void

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
---
 arch/x86/oprofile/op_model_amd.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index cc93046..e95268e 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -132,7 +132,7 @@ op_amd_handle_ibs(struct pt_regs * const regs,
 	struct op_entry entry;
 
 	if (!has_ibs)
-		return 1;
+		return 0;
 
 	if (ibs_config.fetch_enabled) {
 		rdmsrl(MSR_AMD64_IBSFETCHCTL, ctl);
@@ -214,7 +214,10 @@ static void op_amd_stop_ibs(void)
 #else
 
 static inline int op_amd_handle_ibs(struct pt_regs * const regs,
-				    struct op_msrs const * const msrs) { }
+				    struct op_msrs const * const msrs)
+{
+	return 0;
+}
 static inline void op_amd_start_ibs(void) { }
 static inline void op_amd_stop_ibs(void) { }
 
-- 
1.6.0.6

Re: [PATCH -tip] x86: oprofile/op_model_amd.c set return values for op_amd_handle_ibs()

[Robert Richter]

On 18.06.09 17:09:27, Jaswinder Singh Rajput wrote:
> 
> op_amd_handle_ibs() should return 0 when IBS is not present or not defined.
> 
> Fix compilation warning:
>  CC [M]  arch/x86/oprofile/op_model_amd.o
>  arch/x86/oprofile/op_model_amd.c: In function ‘op_amd_handle_ibs’:
>  arch/x86/oprofile/op_model_amd.c:217: warning: no return statement in function returning non-void
> 
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> ---
>  arch/x86/oprofile/op_model_amd.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)

Applied to:

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git master

Thanks Jaswinder,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH -tip] x86: oprofile/op_model_amd.c set return values for op_amd_handle_ibs()

[Andrew Morton]

On Thu, 18 Jun 2009 16:47:31 +0200
Robert Richter <robert.richter@amd.com> wrote:

> On 18.06.09 17:09:27, Jaswinder Singh Rajput wrote:
> > 
> > op_amd_handle_ibs() should return 0 when IBS is not present or not defined.
> > 
> > Fix compilation warning:
> >  CC [M]  arch/x86/oprofile/op_model_amd.o
> >  arch/x86/oprofile/op_model_amd.c: In function ___op_amd_handle_ibs___:
> >  arch/x86/oprofile/op_model_amd.c:217: warning: no return statement in function returning non-void
> > 
> > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > ---
> >  arch/x86/oprofile/op_model_amd.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> Applied to:
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git master
> 

Something has gone badly wrong.  This patch took six weeks to turn up
in linux-next.

Apart from anything else this led me to have to fix something which was
already fixed.

All you guys *saw* that fix and still this didn't prompt anyone to
wonder what had gone wrong.

My fix is better, too.  The op_amd_handle_ibs() return value is
ignored, so it should return void.

Re: [PATCH -tip] x86: oprofile/op_model_amd.c set return values for op_amd_handle_ibs()

[Robert Richter]

On 31.07.09 13:29:27, Andrew Morton wrote:
> On Thu, 18 Jun 2009 16:47:31 +0200
> Robert Richter <robert.richter@amd.com> wrote:
> 
> > On 18.06.09 17:09:27, Jaswinder Singh Rajput wrote:
> > > 
> > > op_amd_handle_ibs() should return 0 when IBS is not present or not defined.
> > > 
> > > Fix compilation warning:
> > >  CC [M]  arch/x86/oprofile/op_model_amd.o
> > >  arch/x86/oprofile/op_model_amd.c: In function ___op_amd_handle_ibs___:
> > >  arch/x86/oprofile/op_model_amd.c:217: warning: no return statement in function returning non-void
> > > 
> > > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > > ---
> > >  arch/x86/oprofile/op_model_amd.c |    7 +++++--
> > >  1 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > Applied to:
> > 
> >  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git master
> > 
> 
> Something has gone badly wrong.  This patch took six weeks to turn up
> in linux-next.

Yes, at that time it was not yet quite clear of how to send oprofile
patches upstream and to linux-next. I was discussing this with Ingo
and we finally agreed to send it over tip to linux-next. It took also
some weeks to find the right workflow and types of branches for this.

All this is sorted out now. See also my current branches in the
oprofile tree:

 http://git.kernel.org/?p=linux/kernel/git/rric/oprofile.git;a=summary

In the last days I added oprofile to linux-next as a backup until Ingo
will be back. The oprofile for-next branch will work also as backup in
the future to make sure, all oprofile patches will be in time in
linux-next. I see the things working now.

Again, I also was not happy with the workflow, but I have fixed it
together with Ingo in the last weeks. That oprofile pops up in
linux-next now is the result of this. So please, see this more as
'things are working now' than 'it took 6 weeks into linux-next'. Also,
it is not too late for this, there are some more weeks to got for the
next merge window.

> 
> Apart from anything else this led me to have to fix something which was
> already fixed.
> 
> All you guys *saw* that fix and still this didn't prompt anyone to
> wonder what had gone wrong.

This went wrong, since the fix didn't make it to tip as fast as the
patch set itself. But things are fixed now and this shouln't happen
again.

> 
> My fix is better, too.  The op_amd_handle_ibs() return value is
> ignored, so it should return void.

There are pros and cons, since the handler actually should tell it did
handle the nmi, it should return something. But for some reason this
has been activly deactivated (maybe a workaround for a bug). And I
decided to not to change the code until I now the reason for it. So
the return value is ignored for now. But anyway, there aren't much
differences in the end. So I will apply your patch instead.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com