Skip to contentSkip to conversation

scsi: add put_device() after device_find_child()

PATCH In review Zhao Hongjiang 3 participants 3 messages May 6, 2013 – May 8, 2013

Subsystems (3)
SCSI iSCSI QLogic QLA4XXX iSCSI driver
lore
Share & tools
lore

Zhao Hongjiang

author
#0 lore raw

PATCH 1/1 scsi: add put_device() after device_find_child()

The qla4xxx_sysfs_ddb_add() function uses device_find_child() but it does not drop the reference of the retrieved child.

Signed-off-by Zhao Hongjiang <[email protected]>
---
 drivers/scsi/qla4xxx/ql4_os.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/qla4xxx/ql4_os.c +1 -0
index a47f999..b378d9e 100644
@@ -5605,6 +5605,7 @@ static int qla4xxx_sysfs_ddb_add(struct Scsi_Host *shost, const char *buf,
 		ql4_printk(KERN_ERR, ha,
 			   "%s: A non-persistent entry %s found\n",
 			   __func__, dev->kobj.name);
+		put_device(dev);
 		goto exit_ddb_add;
 	}
Signature · 1 line
1.7.1
#1 lore raw
Quoted 14 lines · Zhao Hongjiang

The qla4xxx_sysfs_ddb_add() function uses device_find_child() but it does not drop the reference of the retrieved child.

Signed-off-by Zhao Hongjiang <[email protected]>
---
 drivers/scsi/qla4xxx/ql4_os.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/qla4xxx/ql4_os.c +0 -0
index a47f999..b378d9e 100644

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_sysfs_ddb_add(struct Scsi_Host *shost, const char *buf, · line 5611

Quoted 7 lines
drivers/scsi/qla4xxx/ql4_os.c +1 -0
@@ -5605,6 +5605,7 @@ static int qla4xxx_sysfs_ddb_add(struct Scsi_Host *shost, const char *buf,
 		ql4_printk(KERN_ERR, ha,
 			   "%s: A non-persistent entry %s found\n",
 			   __func__, dev->kobj.name);
+		put_device(dev);
 		goto exit_ddb_add;
 	}

Thanks for catching this. I messed up when reviewing it. For some dumb reason I thought it was not doing refcounting.

I think there are a couple bugs from other similar functions. What about the attached patch? It is only compile tested. If it is ok, Vikas, please test.

iscsi class, qla4xxx: fix sess/conn refcounting when find fns are used

This fixes a bug where the iscsi class/driver did not do a put_device when a sess/conn device was found. This also simplifies the interface by not having to pass in some arguments that were duplicated and did not need to be exported.

Signed-off-by Mike Christie <[email protected]>
drivers/scsi/qla4xxx/ql4_os.c +4 -2
index d777332..4d231c1 100644
@@ -5605,6 +5605,7 @@ static int qla4xxx_sysfs_ddb_add(struct Scsi_Host *shost, const char *buf,
 		ql4_printk(KERN_ERR, ha,
 			   "%s: A non-persistent entry %s found\n",
 			   __func__, dev->kobj.name);
+		put_device(dev);
 		goto exit_ddb_add;
 	}
 

@@ -6112,8 +6113,7 @@ qla4xxx_sysfs_ddb_get_param(struct iscsi_bus_flash_session *fnode_sess,
 	int parent_type, parent_index = 0xffff;
 	int rc = 0;
 
-	dev = iscsi_find_flashnode_conn(fnode_sess, NULL,
-					iscsi_is_flashnode_conn_dev);
+	dev = iscsi_find_flashnode_conn(fnode_sess);
 	if (!dev)
 		return -EIO;
 

@@ -6347,6 +6347,8 @@ qla4xxx_sysfs_ddb_get_param(struct iscsi_bus_flash_session *fnode_sess,
 		rc = -ENOSYS;
 		break;
 	}
+
+	put_device(dev);
 	return rc;
 }
drivers/scsi/scsi_transport_iscsi.c +43 -47

217 lines not on this page · raw message

include/scsi/scsi_transport_iscsi.h +2 -6
index 4a58cca..d0f1602 100644
@@ -471,14 +471,10 @@ iscsi_destroy_flashnode_sess(struct iscsi_bus_flash_session *fnode_sess);
 extern void iscsi_destroy_all_flashnode(struct Scsi_Host *shost);
 extern int iscsi_flashnode_bus_match(struct device *dev,
 				     struct device_driver *drv);
-extern int iscsi_is_flashnode_conn_dev(struct device *dev, void *data);
-
 extern struct device *
 iscsi_find_flashnode_sess(struct Scsi_Host *shost, void *data,
 			  int (*fn)(struct device *dev, void *data));
-
 extern struct device *
-iscsi_find_flashnode_conn(struct iscsi_bus_flash_session *fnode_sess,
-			  void *data,
-			  int (*fn)(struct device *dev, void *data));
+iscsi_find_flashnode_conn(struct iscsi_bus_flash_session *fnode_sess);
+
 #endif
Acked-by
#2 lore raw

-----Original Message-----
From: Mike Christie <[email protected]>
Date: Monday 6 May 2013 10:36 PM
To: Zhao Hongjiang <[email protected]>
Cc: "[email protected]" <[email protected]>, Ravi Anand <[email protected]>, Vikas <[email protected]>, scsi <[email protected]>
Subject: Re: [PATCH] scsi: add put_device() after device_find_child()

Quoted 29 lines · Zhao Hongjiang
Quoted 17 lines

The qla4xxx_sysfs_ddb_add() function uses device_find_child() but it does not drop
the reference of the retrieved child.

Signed-off-by Zhao Hongjiang <[email protected]>
---
 drivers/scsi/qla4xxx/ql4_os.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/qla4xxx/ql4_os.c +0 -0
index a47f999..b378d9e 100644

drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_sysfs_ddb_add(struct Scsi_Host · line 5611

Quoted 7 lines
drivers/scsi/qla4xxx/ql4_os.c +1 -0
@@ -5605,6 +5605,7 @@ static int qla4xxx_sysfs_ddb_add(struct Scsi_Host
 *shost, const char *buf,
 		ql4_printk(KERN_ERR, ha,
 			   "%s: A non-persistent entry %s found\n",
 			   __func__, dev->kobj.name);
+		put_device(dev);
 		goto exit_ddb_add;
 	}

Thanks for catching this. I messed up when reviewing it. For some dumb reason I thought it was not doing refcounting.

Quoted 3 lines

I think there are a couple bugs from other similar functions. What about the attached patch? It is only compile tested. If it is ok, Vikas, please test.

We have tested this patch and it looks good in our testing.
I am resending this patch for inclusion with our next patch-set which I am sending in next email.
Thanks for fixing this.

Acked-by Vikas Chaudhary <[email protected]>