Skip to contentSkip to conversation

target: kill CONFIG_TCM_DEBUG_DEV

PATCH In review Christoph Hellwig 2 participants 6 messages Nov 11, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 target: kill CONFIG_TCM_DEBUG_DEV

__iscsi_debug_dev doesn't compile and isn't actually used anywhere. Remove it and the surrounding infrastructure.

drivers/target/lio-target/iscsi_target.c +0 -4
Index: lio-core-2.6/drivers/target/lio-target/iscsi_target.c
===================================================================
@@ -939,10 +939,6 @@ static int iscsi_target_detect(void)
 	spin_lock_init(&iscsi_global->debug_erl_lock);
 #endif /* DEBUG_ERL */
 
-#ifdef DEBUG_DEV
-	spin_lock_init(&iscsi_global->debug_dev_lock);
-#endif
-
 	dir_entry = proc_mkdir("iscsi_target", 0);
 	if (!(dir_entry)) {
 		printk(KERN_ERR "proc_mkdir() failed.\n");
drivers/target/target_core_transport.c +0 -76

88 lines not on this page · raw message

include/target/target_core_base.h +0 -3
Index: lio-core-2.6/include/target/target_core_base.h
===================================================================
@@ -1057,9 +1057,6 @@ struct se_global {
 	spinlock_t		se_tpg_lock;
 	spinlock_t		lu_gps_lock;
 	spinlock_t		plugin_class_lock;
-#ifdef DEBUG_DEV
-	spinlock_t		debug_dev_lock;
-#endif
 } ____cacheline_aligned;
 
 #endif /* TARGET_CORE_BASE_H */
include/target/target_core_transport.h +0 -3
Index: lio-core-2.6/include/target/target_core_transport.h
===================================================================
@@ -132,9 +132,6 @@ struct se_subsystem_api;
 
 extern int init_se_global(void);
 extern void release_se_global(void);
-#ifdef DEBUG_DEV
-extern int __iscsi_debug_dev(struct se_device *);
-#endif
 extern void transport_init_queue_obj(struct se_queue_obj *);
 extern int transport_subsystem_check_init(void);
 extern int transport_subsystem_register(struct se_subsystem_api *);
drivers/target/Kbuild +0 -4
Index: lio-core-2.6/drivers/target/Kbuild
===================================================================
@@ -16,10 +16,6 @@ target_core_mod-y		:= target_core_config
 				   target_core_rd.o \
 				   target_core_mib.o
 
-ifdef CONFIG_TCM_DEBUG_DEV
-EXTRA_CFLAGS			+= -DDEBUG_DEV
-endif
-
 obj-$(CONFIG_TARGET_CORE)	+= target_core_mod.o
 
 # Subsystem modules
drivers/target/Kconfig +0 -7
Index: lio-core-2.6/drivers/target/Kconfig
===================================================================
@@ -5,13 +5,6 @@ config TARGET_CORE
     ---help---
     Say Y or M here to enable the TCM Storage Engine and ConfigFS enabled control path for target_core_mod.  This includes built-in TCM RAMDISK subsystem logic for virtual LUN 0 access
 
-config TCM_DEBUG_DEV
-	tristate "TCM Debug device code"
-	depends on TARGET_CORE
-	default n
-	---help---
-	Say Y here to enable the TCM Debug device code
-
 config TCM_IBLOCK
 	tristate "TCM/IBLOCK Subsystem Plugin for Linux/BLOCK"
 	depends on TARGET_CORE
#1 lore raw

PATCH 2/2 target: remove dead DF_* flags

drivers/target/target_core_iblock.c +0 -1
Index: lio-core-2.6/drivers/target/target_core_iblock.c
===================================================================
@@ -180,7 +180,6 @@ static struct se_device *iblock_create_v
 	dev_limits.hw_queue_depth = IBLOCK_MAX_DEVICE_QUEUE_DEPTH;
 	dev_limits.queue_depth = IBLOCK_DEVICE_QUEUE_DEPTH;
 
-	dev_flags = DF_CLAIMED_BLOCKDEV;
 	ib_dev->ibd_major = MAJOR(bd->bd_dev);
 	ib_dev->ibd_minor = MINOR(bd->bd_dev);
 	ib_dev->ibd_bd = bd;
drivers/target/target_core_rd.c +0 -3
Index: lio-core-2.6/drivers/target/target_core_rd.c
===================================================================
@@ -272,9 +272,6 @@ static struct se_device *rd_create_virtd
 
 	memset(&dev_limits, 0, sizeof(struct se_dev_limits));
 
-	if (rd_dev->rd_direct)
-		dev_flags |= DF_TRANSPORT_DMA_ALLOC;
-
 	if (rd_build_device_space(rd_dev) < 0)
 		goto fail;
 
drivers/target/target_core_transport.c +0 -7
Index: lio-core-2.6/drivers/target/target_core_transport.c
===================================================================
@@ -1983,13 +1983,6 @@ static void core_setup_task_attr_emulati
 		TRANSPORT(dev)->get_device_rev(dev));
 }
 
-/*	transport_add_device_to_core_hba():
- *
- *	Note that some plugins (IBLOCK) will pass device_flags ==
- *	DF_CLAIMED_BLOCKDEV signifying OS that a dependent block_device
- *	has been claimed.  In exception cases we will release said
- *	block_device ourselves.
- */
 struct se_device *transport_add_device_to_core_hba(
 	struct se_hba *hba,
 	struct se_subsystem_api *transport,
include/target/target_core_transport.h +3 -10
Index: lio-core-2.6/include/target/target_core_transport.h
===================================================================
@@ -59,16 +59,9 @@
 /*
  * struct se_device->dev_flags
  */
-#define DF_READAHEAD_ACTIVE                     0x00000001
-#define DF_TRANSPORT_DMA_ALLOC			0x00000002
-#define DF_TRANSPORT_BUF_ALLOC			0x00000004
-#define DF_DEV_DEBUG				0x00000008
-#define DF_CLAIMED_BLOCKDEV			0x00000010
-#define DF_PERSISTENT_CLAIMED_BLOCKDEV		0x00000020
-#define DF_READ_ONLY				0x00000040
-#define DF_SPC3_PERSISTENT_RESERVE		0x00000080
-#define DF_SPC2_RESERVATIONS			0x00000100
-#define DF_SPC2_RESERVATIONS_WITH_ISID		0x00000200
+#define DF_READ_ONLY				0x00000001
+#define DF_SPC2_RESERVATIONS			0x00000002
+#define DF_SPC2_RESERVATIONS_WITH_ISID		0x00000004
 
 /*
  * Used as return values from transport_generic_cmd_sequencer()
#2 lore raw

Re: [PATCH 2/2] target: remove dead DF_* flags

Quoted 20 lines · Christoph Hellwig

Index: lio-core-2.6/drivers/target/target_core_iblock.c
===================================================================

drivers/target/target_core_iblock.c +0 -1
@@ -180,7 +180,6 @@ static struct se_device *iblock_create_v
 	dev_limits.hw_queue_depth = IBLOCK_MAX_DEVICE_QUEUE_DEPTH;
 	dev_limits.queue_depth = IBLOCK_DEVICE_QUEUE_DEPTH;
 
-	dev_flags = DF_CLAIMED_BLOCKDEV;
 	ib_dev->ibd_major = MAJOR(bd->bd_dev);
 	ib_dev->ibd_minor = MINOR(bd->bd_dev);
 	ib_dev->ibd_bd = bd;

Index: lio-core-2.6/drivers/target/target_core_rd.c
===================================================================

drivers/target/target_core_rd.c static struct se_device *rd_create_virtd · line 277

Quoted 9 lines
drivers/target/target_core_rd.c +0 -3
@@ -272,9 +272,6 @@ static struct se_device *rd_create_virtd
 
 	memset(&dev_limits, 0, sizeof(struct se_dev_limits));
 
-	if (rd_dev->rd_direct)
-		dev_flags |= DF_TRANSPORT_DMA_ALLOC;
-
 	if (rd_build_device_space(rd_dev) < 0)
 		goto fail;
 

This is still required by RAMDISK_DR to allow the backend to perform the se_task->task_sg[] mapping from the ramdisk provided struct scatterlists using se_subsystem_api->allocate_dma()

So commited as b9f63304e9, w/o dropping DF_TRANSPORT_DMA_ALLOC..

Quoted 23 lines

Index: lio-core-2.6/drivers/target/target_core_transport.c
===================================================================

drivers/target/target_core_transport.c +0 -7
@@ -1983,13 +1983,6 @@ static void core_setup_task_attr_emulati
 		TRANSPORT(dev)->get_device_rev(dev));
 }
 
-/*	transport_add_device_to_core_hba():
- *
- *	Note that some plugins (IBLOCK) will pass device_flags ==
- *	DF_CLAIMED_BLOCKDEV signifying OS that a dependent block_device
- *	has been claimed.  In exception cases we will release said
- *	block_device ourselves.
- */
 struct se_device *transport_add_device_to_core_hba(
 	struct se_hba *hba,
 	struct se_subsystem_api *transport,

Index: lio-core-2.6/include/target/target_core_transport.h
===================================================================

include/target/target_core_transport.h line 64

Quoted 6 lines
include/target/target_core_transport.h +0 -3
@@ -59,16 +59,9 @@
 /*
  * struct se_device->dev_flags
  */
-#define DF_READAHEAD_ACTIVE                     0x00000001
-#define DF_TRANSPORT_DMA_ALLOC			0x00000002
-#define DF_TRANSPORT_BUF_ALLOC			0x00000004

Btw, just as a historical note for fun..

The DF_TRANSPORT_BUF_ALLOC flag was originally used by a now extinct v2.2/v2.4 backend driver (iscsi_target_ata.c) for the old drivers/ide/ code that required a single contigious buffer to be setup on the non cache coherent MIPS r5900 (the Playstation2 for non linux-mips.org folks) -> r3000 IOP DMA to the single channel PATA expansion adapter.

Anyways, once some aspring MIPS kernel hacker can take the time to figure out how to boot a modern Linux/MIPS kernel on the r5900 we can revist this particular issue again, eg: supporting contigiously allocated buffer for SCF_SCSI_DATA_SG_IO_CDB type descriptors, which still is working the same today for modern iSCSI target fabric code.

Anyways this may still be useful for less obsecure cases than the r5900, but none immediatedly come to mind. ;)

Thanks!

--nab

#3 lore raw

Re: [PATCH 2/2] target: remove dead DF_* flags

Quoted 3 lines · Nicholas A. Bellinger

This is still required by RAMDISK_DR to allow the backend to perform the se_task->task_sg[] mapping from the ramdisk provided struct scatterlists using se_subsystem_api->allocate_dma()

It's not actually used anywhere at all. And if you mean fronted instead of backend above and imply not yet merged - I don't think adding special cases for the ramdisk driver into frontends is a good idea, as it's not really a case worth optimizing for. And even then it's entirely dead and untested code for now.

#4 lore raw

Re: [PATCH 2/2] target: remove dead DF_* flags

Quoted 11 lines · Christoph Hellwig
Quoted 3 lines · Nicholas A. Bellinger

This is still required by RAMDISK_DR to allow the backend to perform the se_task->task_sg[] mapping from the ramdisk provided struct scatterlists using se_subsystem_api->allocate_dma()

It's not actually used anywhere at all. And if you mean fronted instead of backend above and imply not yet merged - I don't think adding special cases for the ramdisk driver into frontends is a good idea, as it's not really a case worth optimizing for. And even then it's entirely dead and untested code for now.

Whoops sorry, transport_dev_get_mem_SG() is simply checking for valid se_subsystem_api->allocate_DMA() and ->free_DMA() function pointers for
the special RAMDISK_DR case. So dropping the unnecessary
DF_TRANSPORT_DMA_ALLOC flag now

Sorry for the extra noise. ;)

Thanks!

#5 lore raw
In reply to Christoph Hellwig
Quoted 5 lines · Christoph Hellwig

__iscsi_debug_dev doesn't compile and isn't actually used anywhere. Remove it and the surrounding infrastructure.

Commited as 67e05b3fde.

Thanks!

Quoted 170 lines

Index: lio-core-2.6/drivers/target/lio-target/iscsi_target.c
===================================================================

drivers/target/lio-target/iscsi_target.c +0 -4
@@ -939,10 +939,6 @@ static int iscsi_target_detect(void)
 	spin_lock_init(&iscsi_global->debug_erl_lock);
 #endif /* DEBUG_ERL */
 
-#ifdef DEBUG_DEV
-	spin_lock_init(&iscsi_global->debug_dev_lock);
-#endif
-
 	dir_entry = proc_mkdir("iscsi_target", 0);
 	if (!(dir_entry)) {
 		printk(KERN_ERR "proc_mkdir() failed.\n");

Index: lio-core-2.6/drivers/target/target_core_transport.c
===================================================================

drivers/target/target_core_transport.c +0 -76
@@ -413,71 +413,6 @@ void release_se_global(void)
 	se_global = NULL;
 }
 
-#ifdef DEBUG_DEV
-
-/* warning FIXME: PLUGIN API TODO */
-int __iscsi_debug_dev(struct se_device *dev)
-{
-	int fail_task = 0;
-	fd_dev_t *fd_dev;
-	iblock_dev_t *ib_dev;
-	rd_dev_t *rd_dev;
-	struct scsi_device *sd;
-
-	spin_lock(&se_global->debug_dev_lock);
-	switch (dev->se_hba->type) {
-	case PSCSI:
-		sd = (struct scsi_device *) dev->dev_ptr;
-		if (dev->dev_flags & DF_DEV_DEBUG) {
-			printk(KERN_INFO "HBA[%u] - Failing PSCSI Task for"
-				" %d/%d/%d\n", dev->se_hba->hba_id,
-				sd->channel, sd->id, sd->lun);
-			fail_task = 1;
-		}
-		break;
-	case IBLOCK:
-		ib_dev = (iblock_dev_t *) dev->dev_ptr;
-		if (dev->dev_flags & DF_DEV_DEBUG) {
-			printk(KERN_INFO "HBA[%u] - Failing IBLOCK Task for"
-				" %u/%u\n", dev->se_hba->hba_id,
-				ib_dev->ibd_major, ib_dev->ibd_minor);
-			fail_task = 1;
-		}
-		break;
-	case FILEIO:
-		fd_dev = (fd_dev_t *) dev->dev_ptr;
-		if (dev->dev_flags & DF_DEV_DEBUG) {
-			printk(KERN_INFO "HBA[%u] - Failing FILEIO Task for"
-				" %u\n", dev->se_hba->hba_id,
-				fd_dev->fd_dev_id);
-			fail_task = 1;
-		}
-		break;
-	case RAMDISK_DR:
-	case RAMDISK_MCP:
-		rd_dev = (rd_dev_t *) dev->dev_ptr;
-		if (dev->dev_flags & DF_DEV_DEBUG) {
-			printk(KERN_INFO "HBA[%u] - Failing RAMDISK Task for"
-				" %u\n", dev->se_hba->hba_id,
-				rd_dev->rd_dev_id);
-			fail_task = 1;
-		}
-		break;
-	default:
-		if (dev->dev_flags & DF_DEV_DEBUG) {
-			printk(KERN_INFO "HBA[%u] - Failing unknown Task\n",
-				dev->se_hba->hba_id);
-			fail_task = 1;
-		}
-		break;
-	}
-	spin_unlock(&se_global->debug_dev_lock);
-
-	return fail_task;
-}
-
-#endif /* DEBUG_DEV */
-
 void transport_init_queue_obj(struct se_queue_obj *qobj)
 {
 	atomic_set(&qobj->queue_cnt, 0);
@@ -1229,17 +1164,6 @@ check_task_stop:
 	}
 	atomic_dec(&T_TASK(cmd)->t_task_cdbs_timeout_left);
 
-#ifdef DEBUG_DEV
-	if (dev) {
-		if (__iscsi_debug_dev(dev) != 0) {
-			success = 0;
-			task->task_scsi_status = 1;
-			cmd->transport_error_status =
-				PYX_TRANSPORT_LU_COMM_FAILURE;
-		}
-	}
-#endif /* DEBUG_DEV */
-
 	/*
 	 * Decrement the outstanding t_task_cdbs_left count.  The last
 	 * struct se_task from struct se_cmd will complete itself into the

Index: lio-core-2.6/include/target/target_core_base.h
===================================================================

include/target/target_core_base.h +0 -3
@@ -1057,9 +1057,6 @@ struct se_global {
 	spinlock_t		se_tpg_lock;
 	spinlock_t		lu_gps_lock;
 	spinlock_t		plugin_class_lock;
-#ifdef DEBUG_DEV
-	spinlock_t		debug_dev_lock;
-#endif
 } ____cacheline_aligned;
 
 #endif /* TARGET_CORE_BASE_H */

Index: lio-core-2.6/include/target/target_core_transport.h
===================================================================

include/target/target_core_transport.h +0 -3
@@ -132,9 +132,6 @@ struct se_subsystem_api;
 
 extern int init_se_global(void);
 extern void release_se_global(void);
-#ifdef DEBUG_DEV
-extern int __iscsi_debug_dev(struct se_device *);
-#endif
 extern void transport_init_queue_obj(struct se_queue_obj *);
 extern int transport_subsystem_check_init(void);
 extern int transport_subsystem_register(struct se_subsystem_api *);

Index: lio-core-2.6/drivers/target/Kbuild
===================================================================

drivers/target/Kbuild +0 -4
@@ -16,10 +16,6 @@ target_core_mod-y		:= target_core_config
 				   target_core_rd.o \
 				   target_core_mib.o
 
-ifdef CONFIG_TCM_DEBUG_DEV
-EXTRA_CFLAGS			+= -DDEBUG_DEV
-endif
-
 obj-$(CONFIG_TARGET_CORE)	+= target_core_mod.o
 
 # Subsystem modules

Index: lio-core-2.6/drivers/target/Kconfig
===================================================================

drivers/target/Kconfig +0 -7
@@ -5,13 +5,6 @@ config TARGET_CORE
     ---help---
     Say Y or M here to enable the TCM Storage Engine and ConfigFS enabled control path for target_core_mod.  This includes built-in TCM RAMDISK subsystem logic for virtual LUN 0 access
 
-config TCM_DEBUG_DEV
-	tristate "TCM Debug device code"
-	depends on TARGET_CORE
-	default n
-	---help---
-	Say Y here to enable the TCM Debug device code
-
 config TCM_IBLOCK
 	tristate "TCM/IBLOCK Subsystem Plugin for Linux/BLOCK"
 	depends on TARGET_CORE