Re: [linux-iscsi-devel] [PATCH] Connfailtimeout exported to sysfs

[Mike C. <mi...@us...>]

Smitha Narayanaswamy (smithan) wrote:
> I have attached the updated patch along with this mail.
> Comments inline:
> 


+static ssize_t
+store_replacement_timeout(struct class_device *class_dev, const char *buf,
+			  size_t count)
+{
+	struct Scsi_Host *shost = class_to_shost(class_dev);
+	struct iscsi_session *session = (struct iscsi_session *)shost->hostdata;
+	int timeout;
+
+	sscanf(buf, "%d\n", &timeout);
+	iscsi_update_replacement_timeout(session, timeout);
+	return count;
+}


check for negative values.


+
  static CLASS_DEVICE_ATTR(shutdown, S_IWUSR, NULL, store_do_shutdown);
  static CLASS_DEVICE_ATTR(drop_session, S_IWUSR, NULL, store_drop_session);
+static CLASS_DEVICE_ATTR(connfailtimeout, S_IWUSR, NULL,
+			 store_replacement_timeout);


You want to make this readable. Just add it with the other timeouts.




+void
+iscsi_update_replacement_timeout(struct iscsi_session *session, int 
timeout)
+{
+	spin_lock(&session->portal_lock);
+	if (timeout < 0) {
+		iscsi_host_err(session, "Cannot set negative timeout value of"
+			       "%d\n", timeout);
+		spin_unlock(&session->portal_lock);
+		return;
+	}
+	del_timer_sync(&session->replacement_timer);
+	session->portal.replacement_timeout =
+	session->replacement_timeout = timeout;
+	spin_lock_bh(&session->task_lock);
+	if ((test_bit(SESSION_ESTABLISHED, &session->control_bits)) ||
+	    (test_bit(SESSION_REPLACEMENT_TIMEDOUT, &session->control_bits)) ||
+	    !timeout) {
+		spin_unlock_bh(&session->task_lock);
+		spin_unlock(&session->portal_lock);
+		return;
+	}
+	spin_unlock_bh(&session->task_lock);
+	timeout *= HZ;
+	mod_timer(&session->replacement_timer,
+		  session->replacement_timer.expires);


this should be
mod_timer(&session->replacement_timer, jiffies + timeout);





+	spin_unlock(&session->portal_lock);
+}
+
  static void
  handle_logout_timeouts(unsigned long data)
  {
@@ -1044,11 +1072,6 @@ iscsi_tx_thread(void *data)
  	struct iscsi_session *session = data;
  	int rc;
  	unsigned long tmo;
-	struct timer_list replacement_timer;
-
-	init_timer(&replacement_timer);
-	replacement_timer.data = (unsigned long)session;
-	replacement_timer.function = replacement_timed_out;

  	current->flags |= PF_MEMALLOC;
  	allow_signal(SIGHUP);
@@ -1061,11 +1084,18 @@ iscsi_tx_thread(void *data)
  	up(&session->tx_blocked);

  	while (!session_kthread_sleep(session)) {
+		spin_lock(&session->portal_lock);
  		tmo = session->replacement_timeout * HZ;
-		if (tmo && session->session_drop_time)
-			mod_timer(&replacement_timer, jiffies + tmo);
+		if (tmo && session->session_drop_time) {
+			del_timer_sync(&session->replacement_timer);
+			mod_timer(&session->replacement_timer,
+				  session->replacement_timer.expires);

this should be
mod_timer(&session->replacement_timer, jiffies + tmo);