2008-07-15 16:15:59

by James Bottomley

[permalink] [raw]
Subject: [GIT PATCH] SCSI part 1

This is the accumulated SCSI patches for the merge window. If your
patch isn't in, don't worry, that'll be for part 2. There was a bit of
an uptick in patches last week, so I haven't got around to reviewing and
merging them all (plus they need to incubate for a bit too).

The patch is available here:

master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6.git

The short changelog is:

Adrian Bunk (2):
make struct scsi_{host,target}_type static
mpt fusion: make struct mpt_proc_root_dir static

Alan Stern (1):
SCSI: remove dev->power.power_state from mesh driver

Alexander Beregalov (1):
scsi_cmnd.h: remove double inclusion of linux/blkdev.h

Benjamin Herrenschmidt (1):
sym53c8xx: Fix bogus sym_que_entry re-implementation of container_of

Boaz Harrosh (1):
iscsi_tcp: Enable any size command

Brian King (2):
ibmvfc: IBM Power Virtual Fibre Channel Adapter Client Driver
sg: Add target reset support

Chandra Seetharaman (9):
scsi_dh: fix kconfig related build errors
scsi_dh: Remove hardware handler infrastructure from dm
scsi_dh: Remove hardware handlers from dm
scsi_dh: Remove dm_pg_init_complete
scsi_dh: Add a single threaded workqueue for initializing paths
scsi_dh: Use SCSI device handler in dm-multipath
scsi_dh: add EMC Clariion device handler
scsi_dh: add lsi rdac device handler
scsi_dh: add infrastructure for SCSI Device Handlers

Christof Schmitt (13):
zfcp: Cleanup external header file
zfcp: Cleanup code in zfcp_erp.c
zfcp: Small QDIO cleanups
zfcp: Fix error checking for ELS ADISC requests
zfcp: Message cleanup
zfcp: Cleanup code in zfcp_ccw
zfcp: Move CFDC code to new file.
zfcp: Move FC code to new file
zfcp: Fix sparse warning by providing new entry in dbf
zfcp: Remove some sparse warnings
zfcp: Fix mempool pointer for GID_PN request allocation
zfcp: sysfs attributes for fabric and channel latencies
zfcp: Track fabric and channel latencies provided by FCP adapter

David S. Miller (1):
esp: Correct chip ID probing sequence.

FUJITA Tomonori (1):
bsg: fix bsg_mutex hang with device removal

Frans Pop (1):
esp: correct module name in Kconfig help for SCSI_SUNESP

Harvey Harrison (3):
scsi: use get_unaligned_* helpers
Replace __FUNCTION__ with __func__ in iscsi_tcp.
aacraid: linit.c make aac_show_serial_number static

Heiko Carstens (1):
zfcp: Fix fsf_status_read return code handling

James Bottomley (2):
fix locking in host use of blk_plug_device()
make use of the residue value

James Smart (5):
lpfc 8.2.7 : Update version to 8.2.7
lpfc 8.2.7 : Miscellaneous Fixes
lpfc 8.2.7 : Rework the worker thread
lpfc 8.2.7 : Discovery Fixes
lpfc 8.2.7 : Change device reset behavior

Mark Salyzyn (2):
aacraid: prevent copy_from_user() BUG!
aacraid: Add Power Management cards to documentation

Martin K. Petersen (5):
lib: Add support for the T10 (SCSI) Data Integrity Field CRC
sd: Move scsi_disk() accessor function to sd.h
sd: Move sd.h header file
Rename scsi_bidi_sdb_cache
scsi_debug: Runtime-configurable sector size

Martin Peschke (7):
zfcp: Remove sysfs attribute port_add
zfcp: remove some __attribute__ ((packed))
zfcp: Refine trace levels of some recovery related events.
zfcp: Add information about interrupt to trace.
zfcp: Rename sbal_curr to sbal_last.
zfcp: Rename sbal_last.
zfcp: Remove field sbal_last from trace record.

Martin Petermann (2):
zfcp: Cleanup of code in zfcp_scsi.c
zfcp: Move status accessors from zfcp to SCSI include file.

Matthew Wilcox (1):
scsi_debug: add support for rotation speed

Mike Christie (28):
iscsi class: fix endpoint leak
iscsi class: update version number
libiscsi, iscsi_tcp, ib_iser: fix setting of can_queue with old tools.
iscsi class: fix refcount leak
libiscsi, iser, tcp: remove recv_lock
libiscsi: fix cmds_max setting
iscsi class: Add session initiatorname and ifacename sysfs attrs.
iscsi_tcp: hook iscsi_tcp into iscsi_endpoint code
iser: Modify iser to take a iscsi_endpoint struct in ep callouts and sessi
iscsi class: add endpoint class
iscsi class: user device_for_each_child instead of duplicating session lis
iser: handle iscsi_cmd_task rename
iscsi_tcp: handle iscsi_cmd_task rename
libiscsi: rename iscsi_cmd_task to iscsi_task
iser: convert ib_iser to support merged tasks
iscsi_tcp: convert iscsi_tcp to support merged tasks
libiscsi: merge iscsi_mgmt_task and iscsi_cmd_task
libiscsi: modify libiscsi so it supports offloaded data paths
libiscsi, iscsi_tcp, iser: add session cmds array accessor
iser: fix handling of scsi cmnds during recovery.
iscsi: modify iscsi printk so it can take driver data pointers
iscsi: remove session/conn_data_size from iscsi_transport
iscsi: add iscsi host helpers
iscsi: remove session and host binding in libiscsi
iscsi class: rename iscsi_host to iscsi_cls_host
iscsi class, iscsi drivers: remove unused iscsi_transport attrs
iscsi class, iscsi_tcp/iser: add host arg to session creation
scsi_dh: add hp sw device handler

Prakash, Sathya (4):
mpt fusion : Adding FAULT Reset polling work
mpt fusion : Setting intial period to 0xFF instead of 0xA
mpt fusion : Updated copyright statment with 2008 included
mpt fusion: Driver version upgrade to 3.04.07

Sven Schuetz (1):
zfcp: Add port_state attribute to sysfs

Swen Schillig (8):
zfcp: zfcp_fsf cleanup.
zfcp: consolidate sysfs things into one file.
zfcp: Cleanup of code in zfcp_aux.c
zfcp: Adapter reopen for large number of unsolicited status
zfcp: wait until adapter is finished with ERP during auto-port
zfcp: Automatically attach remote ports
zfcp: Cleanup qdio code
zfcp: receiving an unsolicted status can lead to I/O stall

And the diffstat:

Documentation/scsi/aacraid.txt | 24
block/bsg.c | 9
drivers/infiniband/ulp/iser/iscsi_iser.c | 356 -
drivers/infiniband/ulp/iser/iscsi_iser.h | 44
drivers/infiniband/ulp/iser/iser_initiator.c | 209 -
drivers/infiniband/ulp/iser/iser_memory.c | 77
drivers/infiniband/ulp/iser/iser_verbs.c | 28
drivers/md/Kconfig | 23
drivers/md/Makefile | 7
drivers/md/dm-emc.c | 345 -
drivers/md/dm-hw-handler.c | 213 -
drivers/md/dm-hw-handler.h | 63
drivers/md/dm-mpath-hp-sw.c | 247 -
drivers/md/dm-mpath-rdac.c | 700 ---
drivers/md/dm-mpath.c | 163
drivers/md/dm-mpath.h | 1
drivers/message/fusion/lsi/mpi.h | 2
drivers/message/fusion/lsi/mpi_cnfg.h | 2
drivers/message/fusion/mptbase.c | 91
drivers/message/fusion/mptbase.h | 17
drivers/message/fusion/mptctl.c | 4
drivers/message/fusion/mptctl.h | 2
drivers/message/fusion/mptdebug.h | 2
drivers/message/fusion/mptfc.c | 2
drivers/message/fusion/mptlan.c | 2
drivers/message/fusion/mptlan.h | 2
drivers/message/fusion/mptsas.c | 2
drivers/message/fusion/mptsas.h | 2
drivers/message/fusion/mptscsih.c | 2
drivers/message/fusion/mptscsih.h | 2
drivers/message/fusion/mptspi.c | 3
drivers/s390/scsi/Makefile | 3
drivers/s390/scsi/zfcp_aux.c | 1689 +-------
drivers/s390/scsi/zfcp_ccw.c | 152
drivers/s390/scsi/zfcp_cfdc.c | 259 +
drivers/s390/scsi/zfcp_dbf.c | 90
drivers/s390/scsi/zfcp_dbf.h | 12
drivers/s390/scsi/zfcp_def.h | 341 -
drivers/s390/scsi/zfcp_erp.c | 3824 +++++-------------
drivers/s390/scsi/zfcp_ext.h | 301 -
drivers/s390/scsi/zfcp_fc.c | 567 ++
drivers/s390/scsi/zfcp_fsf.c | 5573 ++++++++-------------------
drivers/s390/scsi/zfcp_fsf.h | 70
drivers/s390/scsi/zfcp_qdio.c | 811 +--
drivers/s390/scsi/zfcp_scsi.c | 784 ---
drivers/s390/scsi/zfcp_sysfs.c | 496 ++
drivers/s390/scsi/zfcp_sysfs_adapter.c | 270 -
drivers/s390/scsi/zfcp_sysfs_driver.c | 106
drivers/s390/scsi/zfcp_sysfs_port.c | 295 -
drivers/s390/scsi/zfcp_sysfs_unit.c | 167
drivers/scsi/Kconfig | 27
drivers/scsi/Makefile | 2
drivers/scsi/aacraid/commctrl.c | 33
drivers/scsi/aacraid/linit.c | 2
drivers/scsi/device_handler/Kconfig | 32
drivers/scsi/device_handler/Makefile | 7
drivers/scsi/device_handler/scsi_dh.c | 162
drivers/scsi/device_handler/scsi_dh_emc.c | 499 ++
drivers/scsi/device_handler/scsi_dh_hp_sw.c | 202
drivers/scsi/device_handler/scsi_dh_rdac.c | 691 +++
drivers/scsi/esp_scsi.c | 24
drivers/scsi/hosts.c | 2
drivers/scsi/ibmvscsi/Makefile | 1
drivers/scsi/ibmvscsi/ibmvfc.c | 3910 ++++++++++++++++++
drivers/scsi/ibmvscsi/ibmvfc.h | 682 +++
drivers/scsi/iscsi_tcp.c | 514 +-
drivers/scsi/iscsi_tcp.h | 7
drivers/scsi/libiscsi.c | 1359 +++---
drivers/scsi/lpfc/lpfc.h | 21
drivers/scsi/lpfc/lpfc_attr.c | 3
drivers/scsi/lpfc/lpfc_crtn.h | 3
drivers/scsi/lpfc/lpfc_ct.c | 22
drivers/scsi/lpfc/lpfc_els.c | 181
drivers/scsi/lpfc/lpfc_hbadisc.c | 120
drivers/scsi/lpfc/lpfc_init.c | 34
drivers/scsi/lpfc/lpfc_nportdisc.c | 145
drivers/scsi/lpfc/lpfc_scsi.c | 232 -
drivers/scsi/lpfc/lpfc_sli.c | 49
drivers/scsi/lpfc/lpfc_version.h | 2
drivers/scsi/lpfc/lpfc_vport.c | 16
drivers/scsi/mesh.c | 8
drivers/scsi/qla4xxx/ql4_os.c | 8
drivers/scsi/scsi.c | 9
drivers/scsi/scsi_debug.c | 110
drivers/scsi/scsi_error.c | 11
drivers/scsi/scsi_lib.c | 35
drivers/scsi/scsi_scan.c | 2
drivers/scsi/scsi_sysfs.c | 1
drivers/scsi/scsi_transport_iscsi.c | 395 +
drivers/scsi/sd.c | 7
drivers/scsi/sd.h | 62
drivers/scsi/sg.c | 3
drivers/scsi/sym53c8xx_2/sym_misc.h | 4
include/linux/crc-t10dif.h | 8
include/scsi/iscsi_if.h | 93
include/scsi/iscsi_proto.h | 3
include/scsi/libiscsi.h | 107
include/scsi/scsi.h | 18
include/scsi/scsi_cmnd.h | 1
include/scsi/scsi_device.h | 22
include/scsi/scsi_dh.h | 69
include/scsi/scsi_transport_iscsi.h | 91
include/scsi/sd.h | 57
include/scsi/sg.h | 1
lib/Kconfig | 7
lib/Makefile | 1
lib/crc-t10dif.c | 67
107 files changed, 14425 insertions(+), 14215 deletions(-)

James


2008-07-16 10:16:59

by Ingo Molnar

[permalink] [raw]
Subject: [build fix] Re: [GIT PATCH] SCSI part 1


* James Bottomley <[email protected]> wrote:

> zfcp: Move status accessors from zfcp to SCSI include file.

-tip testing found that the upstream build broke in fs/compat_ioctl.c:

----------->
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h:27:25: warning: "BLK_MAX_CDB" is not defined
include/scsi/scsi_cmnd.h:28:3: error: #error MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h: In function ‘scsi_bidi_cmnd’:
include/scsi/scsi_cmnd.h:182: error: implicit declaration of function ‘blk_bidi_rq’
include/scsi/scsi_cmnd.h:183: error: dereferencing pointer to incomplete type
include/scsi/scsi_cmnd.h: In function ‘scsi_in’:
include/scsi/scsi_cmnd.h:189: error: dereferencing pointer to incomplete type
<-----------

with this config:

http://redhat.com/~mingo/misc/config-Wed_Jul_16_11_32_32_CEST_2008.bad

I have bisected it down to:

| feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe is first bad commit
| commit feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe
| Author: Martin Petermann <[email protected]>
| Date: Wed Jul 2 10:56:35 2008 +0200
|
| [SCSI] zfcp: Move status accessors from zfcp to SCSI include file.
|
| Move the accessor functions for the scsi_cmnd status from zfcp to the
| SCSI include file. Change the interface to the functions to pass the
| scsi_cmnd pointer instead of the status pointer.
|
| Signed-off-by: Martin Petermann <[email protected]>
| Signed-off-by: Christof Schmitt <[email protected]>
| Signed-off-by: James Bottomley <[email protected]>

It was not possible to do a clean revert of that change because other
zfcp changes were layered upon it.

The problem is this aspect of that change:

| @@ -9,6 +9,7 @@
| #define _SCSI_SCSI_H
|
| #include <linux/types.h>
| +#include <scsi/scsi_cmnd.h>
|
| /*
| * The maximum number of SG segments that we will put inside a

scsi_cmnd.h depends on symbols defined in blkdev.h. The fix is to
include blkdev.h as well.

The commit below (in tip/out-of-tree) fixes this bug.

Ingo

-------------->
commit 878dfa069329bb302449272ad020ae6f449f693e
Author: Ingo Molnar <[email protected]>
Date: Wed Jul 16 11:56:08 2008 +0200

scsi: fix build error in fs/compat_ioctl.c

-tip testing found that the build broke in fs/compat_ioctl.c:

----------->
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h:27:25: warning: "BLK_MAX_CDB" is not defined
include/scsi/scsi_cmnd.h:28:3: error: #error MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h: In function ‘scsi_bidi_cmnd’:
include/scsi/scsi_cmnd.h:182: error: implicit declaration of function ‘blk_bidi_rq’
include/scsi/scsi_cmnd.h:183: error: dereferencing pointer to incomplete type
include/scsi/scsi_cmnd.h: In function ‘scsi_in’:
include/scsi/scsi_cmnd.h:189: error: dereferencing pointer to incomplete type
<-----------

with this config:

http://redhat.com/~mingo/misc/config-Wed_Jul_16_11_32_32_CEST_2008.bad

I have bisected it down to:

| feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe is first bad commit
| commit feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe
| Author: Martin Petermann <[email protected]>
| Date: Wed Jul 2 10:56:35 2008 +0200
|
| [SCSI] zfcp: Move status accessors from zfcp to SCSI include file.
|
| Move the accessor functions for the scsi_cmnd status from zfcp to the
| SCSI include file. Change the interface to the functions to pass the
| scsi_cmnd pointer instead of the status pointer.
|
| Signed-off-by: Martin Petermann <[email protected]>
| Signed-off-by: Christof Schmitt <[email protected]>
| Signed-off-by: James Bottomley <[email protected]>

The problem is due to this aspect of that change:

| @@ -9,6 +9,7 @@
| #define _SCSI_SCSI_H
|
| #include <linux/types.h>
| +#include <scsi/scsi_cmnd.h>
|
| /*
| * The maximum number of SG segments that we will put inside a

scsi_cmnd.h depends on symbols defined in blkdev.h. The fix is to
include blkdev.h as well.

Signed-off-by: Ingo Molnar <[email protected]>
---
include/scsi/scsi.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 00137a7..2ef4a91 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -9,6 +9,7 @@
#define _SCSI_SCSI_H

#include <linux/types.h>
+#include <linux/blkdev.h>
#include <scsi/scsi_cmnd.h>

/*

2008-07-16 10:34:14

by Ingo Molnar

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1


* Ingo Molnar <[email protected]> wrote:

> The problem is this aspect of that change:
>
> | @@ -9,6 +9,7 @@
> | #define _SCSI_SCSI_H
> |
> | #include <linux/types.h>
> | +#include <scsi/scsi_cmnd.h>
> |
> | /*
> | * The maximum number of SG segments that we will put inside a
>
> 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.

Ingo

----------->
commit 7c2bc6236e6c787ab9962cfc6fad04551d9c4057
Author: Ingo Molnar <[email protected]>
Date: Wed Jul 16 11:56:08 2008 +0200

scsi: fix build error in fs/compat_ioctl.c

-tip testing found that the build broke in fs/compat_ioctl.c:

----------->
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h:27:25: warning: "BLK_MAX_CDB" is not defined
include/scsi/scsi_cmnd.h:28:3: error: #error MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h: In function ‘scsi_bidi_cmnd’:
include/scsi/scsi_cmnd.h:182: error: implicit declaration of function ‘blk_bidi_rq’
include/scsi/scsi_cmnd.h:183: error: dereferencing pointer to incomplete type
include/scsi/scsi_cmnd.h: In function ‘scsi_in’:
include/scsi/scsi_cmnd.h:189: error: dereferencing pointer to incomplete type
<-----------

with this config:

http://redhat.com/~mingo/misc/config-Wed_Jul_16_11_32_32_CEST_2008.bad

I have bisected it down to:

| feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe is first bad commit
| commit feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe
| Author: Martin Petermann <[email protected]>
| Date: Wed Jul 2 10:56:35 2008 +0200
|
| [SCSI] zfcp: Move status accessors from zfcp to SCSI include file.
|
| Move the accessor functions for the scsi_cmnd status from zfcp to the
| SCSI include file. Change the interface to the functions to pass the
| scsi_cmnd pointer instead of the status pointer.
|
| Signed-off-by: Martin Petermann <[email protected]>
| Signed-off-by: Christof Schmitt <[email protected]>
| Signed-off-by: James Bottomley <[email protected]>

The problem is due to this aspect of that change:

| @@ -9,6 +9,7 @@
| #define _SCSI_SCSI_H
|
| #include <linux/types.h>
| +#include <scsi/scsi_cmnd.h>
|
| /*
| * The maximum number of SG segments that we will put inside a

scsi_cmnd.h depends on symbols defined in blkdev.h but those symbols
are not available if !CONFIG_BLOCK.

Only include scsi/scsi_cmnd.h if on CONFIG_BLOCK.
(those methods are not used when CONFIG_BLOCK is off anyway).

Signed-off-by: Ingo Molnar <[email protected]>
---
include/scsi/scsi.h | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 00137a7..b212859 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -9,7 +9,10 @@
#define _SCSI_SCSI_H

#include <linux/types.h>
-#include <scsi/scsi_cmnd.h>
+
+#ifdef CONFIG_BLOCK
+# include <scsi/scsi_cmnd.h>
+#endif

/*
* The maximum number of SG segments that we will put inside a

2008-07-16 13:16:20

by Ingo Molnar

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1


* Ingo Molnar <[email protected]> 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.

Ingo

------------->
commit 21a6d82fe95eced3775fb45ac46102b872db02e5
Author: Ingo Molnar <[email protected]>
Date: Wed Jul 16 11:56:08 2008 +0200

scsi: fix build error in fs/compat_ioctl.c

-tip testing found that the build broke in fs/compat_ioctl.c:

----------->
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h:27:25: warning: "BLK_MAX_CDB" is not defined
include/scsi/scsi_cmnd.h:28:3: error: #error MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB
In file included from include/scsi/scsi.h:12,
from fs/compat_ioctl.c:72:
include/scsi/scsi_cmnd.h: In function ‘scsi_bidi_cmnd’:
include/scsi/scsi_cmnd.h:182: error: implicit declaration of function ‘blk_bidi_rq’
include/scsi/scsi_cmnd.h:183: error: dereferencing pointer to incomplete type
include/scsi/scsi_cmnd.h: In function ‘scsi_in’:
include/scsi/scsi_cmnd.h:189: error: dereferencing pointer to incomplete type
<-----------

with this config:

http://redhat.com/~mingo/misc/config-Wed_Jul_16_11_32_32_CEST_2008.bad

I have bisected it down to:

| feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe is first bad commit
| commit feac6a07c4a3578bffd6769bb4927e8a7e1f3ffe
| Author: Martin Petermann <[email protected]>
| Date: Wed Jul 2 10:56:35 2008 +0200
|
| [SCSI] zfcp: Move status accessors from zfcp to SCSI include file.
|
| Move the accessor functions for the scsi_cmnd status from zfcp to the
| SCSI include file. Change the interface to the functions to pass the
| scsi_cmnd pointer instead of the status pointer.
|
| Signed-off-by: Martin Petermann <[email protected]>
| Signed-off-by: Christof Schmitt <[email protected]>
| Signed-off-by: James Bottomley <[email protected]>

The problem is due to this aspect of that change:

| @@ -9,6 +9,7 @@
| #define _SCSI_SCSI_H
|
| #include <linux/types.h>
| +#include <scsi/scsi_cmnd.h>
|
| /*
| * The maximum number of SG segments that we will put inside a

scsi_cmnd.h depends on symbols defined in blkdev.h but those symbols
are not available if !CONFIG_BLOCK.

Only include scsi/scsi_cmnd.h if on CONFIG_BLOCK.
(those methods are not used when CONFIG_BLOCK is off anyway).

Signed-off-by: Ingo Molnar <[email protected]>
---
include/scsi/scsi.h | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 00137a7..0df6c05 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -9,7 +9,10 @@
#define _SCSI_SCSI_H

#include <linux/types.h>
-#include <scsi/scsi_cmnd.h>
+
+#ifdef CONFIG_BLOCK
+# include <scsi/scsi_cmnd.h>
+#endif

/*
* The maximum number of SG segments that we will put inside a
@@ -426,6 +429,7 @@ struct scsi_lun {
#define driver_byte(result) (((result) >> 24) & 0xff)
#define suggestion(result) (driver_byte(result) & SUGGEST_MASK)

+#ifdef CONFIG_BLOCK
static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)
{
cmd->result |= status << 8;
@@ -440,7 +444,7 @@ static inline void set_driver_byte(struct scsi_cmnd *cmd, char status)
{
cmd->result |= status << 24;
}
-
+#endif

#define sense_class(sense) (((sense) >> 4) & 0x7)
#define sense_error(sense) ((sense) & 0xf)

2008-07-16 13:29:21

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1

On Wed, Jul 16, 2008 at 03:15:43PM +0200, Ingo Molnar wrote:
>
> * Ingo Molnar <[email protected]> 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.

Maybe we should just #ifdef CONFIG_BLOCK the inclusion of scsi/*.h in
fs/compat_ioctl.h?

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2008-07-16 13:52:22

by James Bottomley

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1

On Wed, 2008-07-16 at 15:15 +0200, Ingo Molnar wrote:
> * Ingo Molnar <[email protected]> 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.

James

----

From: Randy Dunlap <[email protected]>

Fix fs/compat_ioctl.c to handle CONFIG_BLOCK=n, CONFIG_SCSI=n
to avoid build errors:

In file included from linux-next-20080708/include/scsi/scsi.h:12,
from linux-next-20080708/fs/compat_ioctl.c:71:
linux-next-20080708/include/scsi/scsi_cmnd.h:27:25: warning:
"BLK_MAX_CDB" is not defined
linux-next-20080708/include/scsi/scsi_cmnd.h:28:3: error: #error
MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB
In file included from linux-next-20080708/include/scsi/scsi.h:12,
from linux-next-20080708/fs/compat_ioctl.c:71:
linux-next-20080708/include/scsi/scsi_cmnd.h: In function
'scsi_bidi_cmnd':
linux-next-20080708/include/scsi/scsi_cmnd.h:182: error: implicit
declaration of function 'blk_bidi_rq'
linux-next-20080708/include/scsi/scsi_cmnd.h:183: error: dereferencing
pointer to incomplete type
linux-next-20080708/include/scsi/scsi_cmnd.h: In function 'scsi_in':
linux-next-20080708/include/scsi/scsi_cmnd.h:189: error: dereferencing
pointer to incomplete type

Signed-off-by: Randy Dunlap <[email protected]>
---
fs/compat_ioctl.c | 6 ++++++
1 file changed, 6 insertions(+)

--- 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)


---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

2008-07-16 14:18:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1


* James Bottomley <[email protected]> wrote:

> On Wed, 2008-07-16 at 15:15 +0200, Ingo Molnar wrote:
> > * Ingo Molnar <[email protected]> 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 <[email protected]>
AuthorDate: Wed Jul 2 10:56:35 2008 +0200
Commit: James Bottomley <[email protected]>
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

2008-07-16 14:28:34

by James Bottomley

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1

On Wed, 2008-07-16 at 16:18 +0200, Ingo Molnar wrote:
> * James Bottomley <[email protected]> wrote:
>
> > On Wed, 2008-07-16 at 15:15 +0200, Ingo Molnar wrote:
> > > * Ingo Molnar <[email protected]> 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 <[email protected]>
> AuthorDate: Wed Jul 2 10:56:35 2008 +0200
> Commit: James Bottomley <[email protected]>
> 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,

It was, that would be this email:

http://marc.info/?l=linux-kernel&m=121555252007662

Perhaps now you might see how even you can miss important stuff on such
a high volume, low signal to noise ratio list ...

> 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.

Not if they miss the email, as you apparently did.

> 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.

The problem being that compat stuff only needs to exist for block ioctls
if block actually exists. If we're going to have a single giant file
for all compat ioctls, then I think we have to fix it the way randy has
done. If you want to separate it into say compat_block.h and simply
#include that into compat.h then we can fix it more cleanly.

James

2008-07-16 14:41:19

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1

On Wed, Jul 16, 2008 at 04:18:05PM +0200, Ingo Molnar wrote:
> btw., about the technical aspects of the solution, i'm not sure i like
> these big #ifdef blocks:

It's partly the nature of the beast. There's already several large
#ifdef blocks in compat_ioctl.c -- CONFIG_NET, CONFIG_VT, CONFIG_SPARC,
etc. I think what would help is a bit more grouping so individual
blocks get bigger.

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2008-07-16 14:45:41

by Ingo Molnar

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1


* James Bottomley <[email protected]> wrote:

> On Wed, 2008-07-16 at 16:18 +0200, Ingo Molnar wrote:
> > * James Bottomley <[email protected]> wrote:
> >
> > > On Wed, 2008-07-16 at 15:15 +0200, Ingo Molnar wrote:
> > > > * Ingo Molnar <[email protected]> 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 <[email protected]>
> > AuthorDate: Wed Jul 2 10:56:35 2008 +0200
> > Commit: James Bottomley <[email protected]>
> > 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,
>
> It was, that would be this email:
>
> http://marc.info/?l=linux-kernel&m=121555252007662

right - i missed it because i limited my search based on the Jul 12
CommitDate. Why is the CommitDate in your commit _after_ the creation of
a fix to it? I have found the patch in linux-next as well now, but under
a different sha1 that was generated on July 7th.

> Perhaps now you might see how even you can miss important stuff on
> such a high volume, low signal to noise ratio list ...

at least for me it's much easier to lose information if it's spread out
to hundreds of low volume lists. (have you ever tried to remain
subscribed on hundreds of lists at once? I have and it's not trivial at
all and the resulting loss of information and mails is frequent.)
Anyway, YMMV.

> > 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.
>
> Not if they miss the email, as you apparently did.
>
> > 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.
>
> The problem being that compat stuff only needs to exist for block
> ioctls if block actually exists. If we're going to have a single
> giant file for all compat ioctls, then I think we have to fix it the
> way randy has done. If you want to separate it into say
> compat_block.h and simply
> #include that into compat.h then we can fix it more cleanly.

well i guess the primary question would be scsi.h: it used to be a
general header file, now it isnt. The ioctl definitions are OK under an
#ifdef i guess. Anyway, i've got no strong opinion, this was just an
observation.

Ingo

2008-07-16 14:46:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1


* Matthew Wilcox <[email protected]> wrote:

> On Wed, Jul 16, 2008 at 04:18:05PM +0200, Ingo Molnar wrote:
> > btw., about the technical aspects of the solution, i'm not sure i like
> > these big #ifdef blocks:
>
> It's partly the nature of the beast. There's already several large
> #ifdef blocks in compat_ioctl.c -- CONFIG_NET, CONFIG_VT,
> #CONFIG_SPARC, etc. I think what would help is a bit more grouping so
> #individual blocks get bigger.

yeah, that's OK - but why is scsi.h #ifdef-ed? For example we can
include blkdev.h without #ifdef CONFIG_BLOCK.

Ingo

2008-07-16 14:54:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1


* James Bottomley <[email protected]> wrote:

> From: Randy Dunlap <[email protected]>
>
> Fix fs/compat_ioctl.c to handle CONFIG_BLOCK=n, CONFIG_SCSI=n
> to avoid build errors:

btw., this patch too solves my build problems.

Tested-by: Ingo Molnar <[email protected]>

Thanks,

Ingo

2008-07-16 14:57:19

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1

On Wed, Jul 16, 2008 at 04:46:02PM +0200, Ingo Molnar wrote:
> yeah, that's OK - but why is scsi.h #ifdef-ed? For example we can
> include blkdev.h without #ifdef CONFIG_BLOCK.

Sure. The important bit comes when deciding which bits need to be
available when CONFIG_BLOCK isn't set. Obviously, there is no SCSI
without block (while sg is a character device, not a block device,
it depends on the block infrastructure to the extent that you couldn't
use it without CONFIG_BLOCK).

While it's not impossible that someone could want the SCSI protocol
and opcodes, status codes, etc. for a different character device, that
seems unlikely. The four opcodes defined in the header seem to be
pretty SCSI-specific and not useful to use on non-scsi devices.
SCSI_IOCTL_GET_PCI looks like it might possibly be useful on more than
just SCSI, but we have better ways (ie sysfs) of determining the same
information in a more general way.

So I think it's fair to put an #ifdef CONFIG_BLOCK right after the
_SCSI_SCSI_H define and close it right at the end of the file.

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2008-07-16 14:59:32

by Ingo Molnar

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1


* Matthew Wilcox <[email protected]> wrote:

> On Wed, Jul 16, 2008 at 04:46:02PM +0200, Ingo Molnar wrote:
> > yeah, that's OK - but why is scsi.h #ifdef-ed? For example we can
> > include blkdev.h without #ifdef CONFIG_BLOCK.
>
> Sure. The important bit comes when deciding which bits need to be
> available when CONFIG_BLOCK isn't set. Obviously, there is no SCSI
> without block (while sg is a character device, not a block device, it
> depends on the block infrastructure to the extent that you couldn't
> use it without CONFIG_BLOCK).
>
> While it's not impossible that someone could want the SCSI protocol
> and opcodes, status codes, etc. for a different character device, that
> seems unlikely. The four opcodes defined in the header seem to be
> pretty SCSI-specific and not useful to use on non-scsi devices.
> SCSI_IOCTL_GET_PCI looks like it might possibly be useful on more than
> just SCSI, but we have better ways (ie sysfs) of determining the same
> information in a more general way.
>
> So I think it's fair to put an #ifdef CONFIG_BLOCK right after the
> _SCSI_SCSI_H define and close it right at the end of the file.

yes, that's what blkdev.h does, and that's what i was suggesting we do,
instead of #ifdef-ing around scsi.h use in fs/compat_ioctl.c.

Ingo

2008-07-16 15:12:20

by James Bottomley

[permalink] [raw]
Subject: Re: [build fix] Re: [GIT PATCH] SCSI part 1

On Wed, 2008-07-16 at 16:45 +0200, Ingo Molnar wrote:
> * James Bottomley <[email protected]> wrote:
>
> > On Wed, 2008-07-16 at 16:18 +0200, Ingo Molnar wrote:
> > > * James Bottomley <[email protected]> wrote:
> > >
> > > > On Wed, 2008-07-16 at 15:15 +0200, Ingo Molnar wrote:
> > > > > * Ingo Molnar <[email protected]> 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 <[email protected]>
> > > AuthorDate: Wed Jul 2 10:56:35 2008 +0200
> > > Commit: James Bottomley <[email protected]>
> > > 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,
> >
> > It was, that would be this email:
> >
> > http://marc.info/?l=linux-kernel&m=121555252007662
>
> right - i missed it because i limited my search based on the Jul 12
> CommitDate. Why is the CommitDate in your commit _after_ the creation of
> a fix to it? I have found the patch in linux-next as well now, but under
> a different sha1 that was generated on July 7th.

Because my tree got rebased; some of the patches needed to be moved to
immediate fixes.

James