Re: [PATCH 1/1] iscsi: fix regression caused by session lock patch

2016-11-08 Thread Chris Leech
On Mon, Nov 07, 2016 at 04:23:10PM -0200, Guilherme G. Piccoli wrote:
>
> Sure! Count on us to test any patches. I guess the first step is to
> reproduce on upstream right? We haven't tested specifically this
> scenario for long time. Will try to reproduce on 4.9-rc4 and update here.

Great, I'm looking forward to hearing the result.

Assuming it reproduces, I don't think this level of fine grained locking
is necessarily the best solution, but it may help confirm the problem.
Especially if the WARN_ONCE I slipped in here triggers.

- Chris

---

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index f9b6fba..fbd18ab 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -560,8 +560,12 @@ static void iscsi_complete_task(struct iscsi_task *task, 
int state)
WARN_ON_ONCE(task->state == ISCSI_TASK_FREE);
task->state = state;
 
-   if (!list_empty(&task->running))
+   spin_lock_bh(&conn->taskqueuelock);
+   if (!list_empty(&task->running)) {
+   WARN_ONCE(1, "iscsi_complete_task while task on list");
list_del_init(&task->running);
+   }
+   spin_unlock_bh(&conn->taskqueuelock);
 
if (conn->task == task)
conn->task = NULL;
@@ -783,7 +787,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct 
iscsi_hdr *hdr,
if (session->tt->xmit_task(task))
goto free_task;
} else {
+   spin_lock_bh(&conn->taskqueuelock);
list_add_tail(&task->running, &conn->mgmtqueue);
+   spin_unlock_bh(&conn->taskqueuelock);
iscsi_conn_queue_work(conn);
}
 
@@ -1474,8 +1480,10 @@ void iscsi_requeue_task(struct iscsi_task *task)
 * this may be on the requeue list already if the xmit_task callout
 * is handling the r2ts while we are adding new ones
 */
+   spin_lock_bh(&conn->taskqueuelock);
if (list_empty(&task->running))
list_add_tail(&task->running, &conn->requeue);
+   spin_unlock_bh(&conn->taskqueuelock);
iscsi_conn_queue_work(conn);
 }
 EXPORT_SYMBOL_GPL(iscsi_requeue_task);
@@ -1512,22 +1520,26 @@ static int iscsi_data_xmit(struct iscsi_conn *conn)
 * only have one nop-out as a ping from us and targets should not
 * overflow us with nop-ins
 */
+   spin_lock_bh(&conn->taskqueuelock);
 check_mgmt:
while (!list_empty(&conn->mgmtqueue)) {
conn->task = list_entry(conn->mgmtqueue.next,
 struct iscsi_task, running);
list_del_init(&conn->task->running);
+   spin_unlock_bh(&conn->taskqueuelock);
if (iscsi_prep_mgmt_task(conn, conn->task)) {
/* regular RX path uses back_lock */
spin_lock_bh(&conn->session->back_lock);
__iscsi_put_task(conn->task);
spin_unlock_bh(&conn->session->back_lock);
conn->task = NULL;
+   spin_lock_bh(&conn->taskqueuelock);
continue;
}
rc = iscsi_xmit_task(conn);
if (rc)
goto done;
+   spin_lock_bh(&conn->taskqueuelock);
}
 
/* process pending command queue */
@@ -1535,19 +1547,24 @@ static int iscsi_data_xmit(struct iscsi_conn *conn)
conn->task = list_entry(conn->cmdqueue.next, struct iscsi_task,
running);
list_del_init(&conn->task->running);
+   spin_unlock_bh(&conn->taskqueuelock);
if (conn->session->state == ISCSI_STATE_LOGGING_OUT) {
fail_scsi_task(conn->task, DID_IMM_RETRY);
+   spin_lock_bh(&conn->taskqueuelock);
continue;
}
rc = iscsi_prep_scsi_cmd_pdu(conn->task);
if (rc) {
if (rc == -ENOMEM || rc == -EACCES) {
+   spin_lock_bh(&conn->taskqueuelock);
list_add_tail(&conn->task->running,
  &conn->cmdqueue);
conn->task = NULL;
+   spin_unlock_bh(&conn->taskqueuelock);
goto done;
} else
fail_scsi_task(conn->task, DID_ABORT);
+   spin_lock_bh(&conn->taskqueuelock);
continue;
}
rc = iscsi_xmit_task(conn);
@@ -1558,6 +1575,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn)
 * we need to check the mgmt queue for nops that need to
 * be sent to aviod starvation
 */
+   spin_lock_bh(&conn->taskqueuelock);
 

why certain function name prefix of “_ _” (double underscore)

2016-11-08 Thread Raghu Murugesan
I am reading the source code of open-iscsi. In source files, I see few 
functions start with prefix "__". Whats the reason behind naming a function 
with double underscore as prefix in general in C language?

Example: In the file usr/iscsi_sysfs.c, the function name `static int 
__get_host_no_from_hwaddress(void *data, struct host_info *info)`

Thanks for reading the post

- Raghu

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Open-Iscsi working

2016-11-08 Thread Raghu Murugesan
I want to understand how open-iscsi implements the RFC3720. I read RFC3720 
to understand how iscsi works. 

But I could understand how this RFC3720 is implemented in open-iscsi code. 
There are a lot of files with few thousand line of code which making things 
difficult to understand for someone new like me.

Can anyone give me some suggestion on how understand open-iscsi 
implementation. Any links or resources to read?

I know this sounds dumb. But I really need this help since I am new to this 
field.

Thanks for reading this post. 

- Raghu

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.