[PATCH] libsrp: fix compile failure

[PATCH] libsrp: fix compile failure

[James Bottomley]

commit 45465487897a1c6d508b14b904dc5777f7ec7e04
Author: Stefani Seibold <stefani@seibold.net>
Date:   Mon Dec 21 14:37:26 2009 -0800

    kfifo: move struct kfifo in place

Caused a compile failure in ibmvscsitgt.c because it changed a pointer
to kfifo in the libsrp.h structure to a direct inclusion without
including <linux/kfifo.h>.  The fix is simple, just add the include, but
how did this happen?  This change, introduced at -rc2, hardly looks like
a bug fix, and it clearly didn't go through linux-next, which would have
picked up this compile failure (it only occurs on ppc because of the ibm
virtual scsi target).

James

---

diff --git a/include/scsi/libsrp.h b/include/scsi/libsrp.h
index 07e3add..f4105c9 100644
--- a/include/scsi/libsrp.h
+++ b/include/scsi/libsrp.h
@@ -2,6 +2,7 @@
 #define __LIBSRP_H__
 
 #include <linux/list.h>
+#include <linux/kfifo.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_host.h>
 #include <scsi/srp.h>

Re: [PATCH] libsrp: fix compile failure

[Linus Torvalds]

On Wed, 30 Dec 2009, James Bottomley wrote:
>
> The fix is simple, just add the include, but how did this happen?  This 
> change, introduced at -rc2, hardly looks like a bug fix, and it clearly 
> didn't go through linux-next, which would have picked up this compile 
> failure (it only occurs on ppc because of the ibm virtual scsi target).

It came through Andrew - and apparently parts of Andrews chain weren't in 
next. Don't know why.

> James

Sign-off?

		Linus

Re: [PATCH] libsrp: fix compile failure

[James Bottomley]

On Wed, 2009-12-30 at 11:33 -0800, Linus Torvalds wrote:
> 
> On Wed, 30 Dec 2009, James Bottomley wrote:
> >
> > The fix is simple, just add the include, but how did this happen?  This 
> > change, introduced at -rc2, hardly looks like a bug fix, and it clearly 
> > didn't go through linux-next, which would have picked up this compile 
> > failure (it only occurs on ppc because of the ibm virtual scsi target).
> 
> It came through Andrew - and apparently parts of Andrews chain weren't in 
> next. Don't know why.
> 
> > James
> 
> Sign-off?

Sorry, forgot ... 

Signed-off-by: James Bottomley <James.Bottomley@suse.de>

James

Re: [PATCH] libsrp: fix compile failure

[Jiri Kosina]

On Wed, 30 Dec 2009, Linus Torvalds wrote:

> > The fix is simple, just add the include, but how did this happen?  This 
> > change, introduced at -rc2, hardly looks like a bug fix, and it clearly 
> > didn't go through linux-next, which would have picked up this compile 
> > failure (it only occurs on ppc because of the ibm virtual scsi target).
> 
> It came through Andrew - and apparently parts of Andrews chain weren't in 
> next. Don't know why.

Uhm ... are they supposed to be? -mm is being built on top of -next, not 
vice versa, right?

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] libsrp: fix compile failure

[James Bottomley]

On Mon, 2010-01-04 at 16:21 +0100, Jiri Kosina wrote:
> On Wed, 30 Dec 2009, Linus Torvalds wrote:
> 
> > > The fix is simple, just add the include, but how did this happen?  This 
> > > change, introduced at -rc2, hardly looks like a bug fix, and it clearly 
> > > didn't go through linux-next, which would have picked up this compile 
> > > failure (it only occurs on ppc because of the ibm virtual scsi target).
> > 
> > It came through Andrew - and apparently parts of Andrews chain weren't in 
> > next. Don't know why.
> 
> Uhm ... are they supposed to be? -mm is being built on top of -next, not 
> vice versa, right?

Well, the fact that the compile failure wasn't detected before it went
upstream should answer that ...

But to be more specific: linux-next is our integration tree (and also
the obscure architecture compile tree).  To ensure the best possible
integration, every tree should be built and tested in linux-next at
least once before it goes to Linus.  There were originally technical
reasons why -mm wasn't in ... I just thought they'd been fixed by now.

James

Re: [PATCH] libsrp: fix compile failure

[Alan Cox]

> least once before it goes to Linus.  There were originally technical
> reasons why -mm wasn't in ... I just thought they'd been fixed by now.

No - mm also caused problems with the kfifo merge of a core API change
that didn't go via -next.

Alan

Re: [PATCH] libsrp: fix compile failure

[Stefani Seibold]

Am Montag, den 04.01.2010, 17:35 +0000 schrieb Alan Cox:
> > least once before it goes to Linus.  There were originally technical
> > reasons why -mm wasn't in ... I just thought they'd been fixed by now.
> 
> No - mm also caused problems with the kfifo merge of a core API change
> that didn't go via -next.
> 
> Alan

I tried my best to port everything to the new kfifo API. But it was not
possible to check it against every architecture. x86 and x86_64 was
compile clean. I think the missing #include was not a big thing.

Stefani

Re: [PATCH] libsrp: fix compile failure

[James Bottomley]

On Mon, 2010-01-04 at 20:24 +0100, Stefani Seibold wrote:
> Am Montag, den 04.01.2010, 17:35 +0000 schrieb Alan Cox:
> > > least once before it goes to Linus.  There were originally technical
> > > reasons why -mm wasn't in ... I just thought they'd been fixed by now.
> > 
> > No - mm also caused problems with the kfifo merge of a core API change
> > that didn't go via -next.
> > 
> > Alan
> 
> I tried my best to port everything to the new kfifo API. But it was not
> possible to check it against every architecture.

That's what linux-next is for:  to make at least this compile checking
against all of our architectures more of a reality.  It's still missing
bits, and some of the ports (*cough* parisc *cough*) compile against
linux-next rather infrequently, but it's far better than nothing.

>  x86 and x86_64 was
> compile clean. I think the missing #include was not a big thing.

It's easily fixable, yes ... breaking the build *is* a pretty big thing
because of the fallout it causes (for ppc: about ~100 instant WTF
moments followed by grubbing around in git to find the cause).

It's also an indicator of future problems.  Realistically, all API
changes should go into linux-next both to prevent this sort of thing
from happening and to see if we have any other pending conflicts that
may cause problems.

James

Re: [PATCH] libsrp: fix compile failure

[Jiri Kosina]

[ added Stephen to CC ]

On Mon, 4 Jan 2010, James Bottomley wrote:

> Well, the fact that the compile failure wasn't detected before it went 
> upstream should answer that ...
> 
> But to be more specific: linux-next is our integration tree (and also
> the obscure architecture compile tree).  To ensure the best possible
> integration, every tree should be built and tested in linux-next at
> least once before it goes to Linus.  There were originally technical
> reasons why -mm wasn't in ... I just thought they'd been fixed by now.

/me checks ...

Yes, it indeed is that way -- Andew pulls whole linux-next as one of the 
patches into -mm series.

To make linux-next really working the way it is intended to work we need 
to have -mm part of it, as it is used as a last point for a non-trivial 
amount of patches before they enter Linus' tree.

Andrew, why do we have the current setup, and not the other way around?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] libsrp: fix compile failure

[Andrew Morton]

On Mon, 4 Jan 2010 22:12:26 +0100 (CET)
Jiri Kosina <jkosina@suse.cz> wrote:

> 
> [ added Stephen to CC ]
> 
> On Mon, 4 Jan 2010, James Bottomley wrote:
> 
> > Well, the fact that the compile failure wasn't detected before it went 
> > upstream should answer that ...
> > 
> > But to be more specific: linux-next is our integration tree (and also
> > the obscure architecture compile tree).  To ensure the best possible
> > integration, every tree should be built and tested in linux-next at
> > least once before it goes to Linus.  There were originally technical
> > reasons why -mm wasn't in ... I just thought they'd been fixed by now.
> 
> /me checks ...
> 
> Yes, it indeed is that way -- Andew pulls whole linux-next as one of the 
> patches into -mm series.
> 
> To make linux-next really working the way it is intended to work we need 
> to have -mm part of it, as it is used as a last point for a non-trivial 
> amount of patches before they enter Linus' tree.
> 
> Andrew, why do we have the current setup, and not the other way around?
> 

Because I suck.  I haven't yet got around to feeding -mm into
linux-next.  It's a bit tricky, because -mm is based on linux-next. 
Probably we'll address this by adding a "linux-next before the mm bits"
marker to linux-next.

Re: [PATCH] libsrp: fix compile failure

[Jiri Kosina]

On Mon, 4 Jan 2010, Andrew Morton wrote:

> > Yes, it indeed is that way -- Andew pulls whole linux-next as one of the 
> > patches into -mm series.
> > 
> > To make linux-next really working the way it is intended to work we need 
> > to have -mm part of it, as it is used as a last point for a non-trivial 
> > amount of patches before they enter Linus' tree.
> > 
> > Andrew, why do we have the current setup, and not the other way around?
> 
> Because I suck.  I haven't yet got around to feeding -mm into 
> linux-next.  It's a bit tricky, because -mm is based on linux-next. 
> Probably we'll address this by adding a "linux-next before the mm bits" 
> marker to linux-next.

Yup. That should work and would be really helpful.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] libsrp: fix compile failure

[Alan Cox]

On Mon, 04 Jan 2010 20:24:14 +0100
Stefani Seibold <stefani@seibold.net> wrote:

> Am Montag, den 04.01.2010, 17:35 +0000 schrieb Alan Cox:
> > > least once before it goes to Linus.  There were originally technical
> > > reasons why -mm wasn't in ... I just thought they'd been fixed by now.
> > 
> > No - mm also caused problems with the kfifo merge of a core API change
> > that didn't go via -next.
> > 
> > Alan
> 
> I tried my best to port everything to the new kfifo API. But it was not
> possible to check it against every architecture. x86 and x86_64 was
> compile clean. I think the missing #include was not a big thing.

Not your fault - -next is there so stuff is visible for merging and to
avoid collisions. Andrew not going via -next broke various pending
patches.

Conversion is easy enough, it just wasn't visible at the right time.