Re: [build fix] Re: [GIT PATCH] SCSI part 1

From: Ingo Molnar
Date: Wed Jul 16 2008 - 10:18:30 EST



* James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:

> On Wed, 2008-07-16 at 15:15 +0200, Ingo Molnar wrote:
> > * Ingo Molnar <mingo@xxxxxxx> wrote:
> >
> > > > scsi_cmnd.h depends on symbols defined in blkdev.h. The fix is to
> > > > include blkdev.h as well.
> > >
> > > that wont work - a better replacement fix is the one below. The
> > > problem is that scsi.h is included even on !CONFIG_BLOCK and then the
> > > BLK_MAX_CDB symbol is meaningless.
> >
> > -v3 .. the new methods need to be under #ifdef CONFIG_BLOCK as well.
> > Note my patch is just a quick RFC, this can probably be done
> > cleaner.
>
> Erm, Ingo, if you'd just follow linux-next instead of your own tree,
> you'd see there's already a fix for this.

Erm, no. In the merge window i follow upstream -git, not "my tree", and
i searched lkml for the build failure signature and it had nothing
there. Then i looked at the commit and it said that it was created just
1 day before the merge window started:

commit feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe
Author: Martin Petermann <martin@xxxxxxxxxxxxxxxxxx>
AuthorDate: Wed Jul 2 10:56:35 2008 +0200
Commit: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
CommitDate: Sat Jul 12 08:22:34 2008 -0500
^^^^^^

So i didnt even think of it having hit linux-next so i didnt look into
the linux-next archives. lkml should have been Cc:-ed in this case,
that's where people look for in case of upstream breakages. You would
have saved me some effort via that - please try to do it in the future,
it's very helpful to testers.

btw., about the technical aspects of the solution, i'm not sure i like
these big #ifdef blocks:

> --- linux-next-20080708.orig/fs/compat_ioctl.c
> +++ linux-next-20080708/fs/compat_ioctl.c
> @@ -68,9 +68,11 @@
> #include <linux/capi.h>
> #include <linux/gigaset_dev.h>
>
> +#ifdef CONFIG_BLOCK
> #include <scsi/scsi.h>
> #include <scsi/scsi_ioctl.h>
> #include <scsi/sg.h>
> +#endif
>
> #include <asm/uaccess.h>
> #include <linux/ethtool.h>
> @@ -1965,6 +1967,7 @@ COMPATIBLE_IOCTL(GIO_UNISCRNMAP)
> COMPATIBLE_IOCTL(PIO_UNISCRNMAP)
> COMPATIBLE_IOCTL(PIO_FONTRESET)
> COMPATIBLE_IOCTL(PIO_UNIMAPCLR)
> +#ifdef CONFIG_BLOCK
> /* Big S */
> COMPATIBLE_IOCTL(SCSI_IOCTL_GET_IDLUN)
> COMPATIBLE_IOCTL(SCSI_IOCTL_DOORLOCK)
> @@ -1974,6 +1977,7 @@ COMPATIBLE_IOCTL(SCSI_IOCTL_GET_BUS_NUMB
> COMPATIBLE_IOCTL(SCSI_IOCTL_SEND_COMMAND)
> COMPATIBLE_IOCTL(SCSI_IOCTL_PROBE_HOST)
> COMPATIBLE_IOCTL(SCSI_IOCTL_GET_PCI)
> +#endif
> /* Big T */
> COMPATIBLE_IOCTL(TUNSETNOCSUM)
> COMPATIBLE_IOCTL(TUNSETDEBUG)
> @@ -2044,6 +2048,7 @@ COMPATIBLE_IOCTL(SIOCGIFVLAN)
> COMPATIBLE_IOCTL(SIOCSIFVLAN)
> COMPATIBLE_IOCTL(SIOCBRADDBR)
> COMPATIBLE_IOCTL(SIOCBRDELBR)
> +#ifdef CONFIG_BLOCK
> /* SG stuff */
> COMPATIBLE_IOCTL(SG_SET_TIMEOUT)
> COMPATIBLE_IOCTL(SG_GET_TIMEOUT)
> @@ -2068,6 +2073,7 @@ COMPATIBLE_IOCTL(SG_SCSI_RESET)
> COMPATIBLE_IOCTL(SG_GET_REQUEST_TABLE)
> COMPATIBLE_IOCTL(SG_SET_KEEP_ORPHAN)
> COMPATIBLE_IOCTL(SG_GET_KEEP_ORPHAN)
> +#endif
> /* PPP stuff */
> COMPATIBLE_IOCTL(PPPIOCGFLAGS)
> COMPATIBLE_IOCTL(PPPIOCSFLAGS)

the clean solution we use everywhere else is to push such #ifdefs into
the headers, to make them generally includable. For example you can
include lockdep.h even if you dont have lockdep enabled, you can include
smp.h even on UP-only files, etc. etc.

Ingo
--
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/