Re: [PATCH -tip] x86: oprofile/op_model_amd.c set return valuesfor 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@xxxxxxx> 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@xxxxxxxxx>
> > > ---
> > >  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@xxxxxxx

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/