[stgt] [PATCH 2/2] iscsi user-initiated disconnect fix: scheduling mtask processing

Alexander Nezhinsky alexandern at Voltaire.COM
Sun Nov 7 18:00:22 CET 2010


Fixes another bug with user-initiated disconnect. It led to segfaults
when deleting an iscsi connection during concurrent I/O.

Processing of managements tasks (mtasks) has been performed in the main event loop.
When epoll_wait() returns, it saves all signaled file descriptors in an array, 
and those are processed one by one. There is a tacit assumption that every event 
is processed separately and independently of other events. Still, in case of mtasks
there may be a dependency between handling an mtask and the remaining events.
When a management request asking for connection removal arrives during a heavy
traffic, there is a high probability that the tcp socket of the iscsi connection
is also signaled. If the management fd is processed first (which is guaranteed,
as it is created and added to epoll first), then the connection and all its 
associated data incl. the tcp-related event itself are destroyed.
When the turn of this deleted tcp-related event in the epoll_wait-filled array
comes, we end up referencing garbage.

The solution implemented here is to schedule an event that calls the original
mtask-processing code instead of processing it on the spot.
Because the scheduled events are serviced after all epoll_wait() events are handled,
this guarantees that the connection is destroyed when no epoll_wait-detected fds 
are waiting. After that epoll_wait() won't detect anything related to the connection,
as its fd is already removed from epoll records.

---
 usr/mgmt.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/usr/mgmt.c b/usr/mgmt.c
index 5a6a03b..f29228a 100644
--- a/usr/mgmt.c
+++ b/usr/mgmt.c
@@ -53,6 +53,8 @@ struct mgmt_task {
 	int done;
 	char *buf;
 	int bsize;
+	int fd;
+	struct event_data tev;
 	struct tgtadm_req req;
 	struct tgtadm_rsp rsp;
 /* 	struct tgt_work work; */
@@ -411,11 +413,12 @@ static int ipc_perm(int fd)
 	return 0;
 }
 
-static void mtask_handler(int fd, int events, void *data)
+void mtask_sched_handler(struct event_data *tev)
 {
 	int err, len;
 	char *p;
-	struct mgmt_task *mtask = data;
+	struct mgmt_task *mtask = tev->data;
+	int fd = mtask->fd;
 	struct tgtadm_req *req = &mtask->req;
 	struct tgtadm_rsp *rsp = &mtask->rsp;
 
@@ -506,6 +509,15 @@ out:
 	close(fd);
 }
 
+static void mtask_handler(int fd, int events, void *data)
+{
+	struct mgmt_task *mtask = data;
+	
+	mtask->fd = fd;
+	tgt_init_sched_event(&mtask->tev, mtask_sched_handler, mtask);
+	tgt_add_sched_event(&mtask->tev);
+}
+
 #define BUFSIZE 1024
 
 static void mgmt_event_handler(int accept_fd, int events, void *data)
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



More information about the stgt mailing list