[stgt] help tgt segfault

[FUJITA Tomonori]

On Mon, 22 Dec 2008 21:52:26 +0100
Tomasz Chmielewski <mangoo at wpkg.org> wrote:

> FUJITA Tomonori schrieb:
> 
> > Looks like tgtd works fine. Let me know the results later. If it works
> > fine, I'll clean up and merge the patch.
> 
> I was running the tests for about 8 hours and everything worked fine.
> 
> No more I/O errors from open-iscsi side as well.

Thanks a lot,

I've merged the following patch:

=
From: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
Subject: [PATCH] iscsi: fix session reinstatement ref-count mess-up

login_security_done calls conn_close() in case of session
reinstatement. if conn_close() was already called against a
connection, it messes up the connection ref-counting. conn_close()
must be called only once for a connection.

This patch introduces the closed to struct iscsi_connection to avoid
multiple calls of conn_close but it should be a new state.

Thanks to Tomasz Chmielewski <mangoo at wpkg.org> for helping me fix the
bugs.

Reported-by: Tomasz Chmielewski <mangoo at wpkg.org>
Tested-by: Tomasz Chmielewski <mangoo at wpkg.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
---
 usr/iscsi/conn.c      |   17 +++++++++++++++--
 usr/iscsi/iscsi_tcp.c |    2 +-
 usr/iscsi/iscsid.h    |    4 ++++
 usr/tgtd.c            |    6 +++++-
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/usr/iscsi/conn.c b/usr/iscsi/conn.c
index c205397..e4b431e 100644
--- a/usr/iscsi/conn.c
+++ b/usr/iscsi/conn.c
@@ -82,15 +82,28 @@ void conn_exit(struct iscsi_connection *conn)
 void conn_close(struct iscsi_connection *conn)
 {
 	struct iscsi_task *task, *tmp;
+	int ret;
 
-	conn->tp->ep_close(conn);
+	if (conn->closed) {
+		eprintf("already closed %p %u\n", conn, conn->refcount);
+		return;
+	}
+
+	conn->closed = 1;
 
-	eprintf("connection closed %p %u\n", conn, conn->refcount);
+	ret = conn->tp->ep_close(conn);
+	if (ret)
+		eprintf("failed to close a connection, %p %u %s\n",
+			conn, conn->refcount, strerror(errno));
+	else
+		eprintf("connection closed, %p %u\n", conn, conn->refcount);
 
 	/* may not have been in FFP yet */
 	if (!conn->session)
 		goto done;
 
+	eprintf("sesson %p %d\n", conn->session, conn->session->refcount);
+
 	/*
 	 * We just closed the ep so we are not going to send/recv anything.
 	 * Just free these up since they are not going to complete.
diff --git a/usr/iscsi/iscsi_tcp.c b/usr/iscsi/iscsi_tcp.c
index 2320b3e..edc4e86 100644
--- a/usr/iscsi/iscsi_tcp.c
+++ b/usr/iscsi/iscsi_tcp.c
@@ -164,8 +164,8 @@ static void iscsi_tcp_event_handler(int fd, int events, void *data)
 		iscsi_tx_handler(conn);
 
 	if (conn->state == STATE_CLOSE) {
+		dprintf("connection closed %p\n", conn);
 		conn_close(conn);
-		dprintf("connection closed\n");
 	}
 }
 
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index c03263a..4a8deb9 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -130,6 +130,10 @@ struct iscsi_task {
 
 struct iscsi_connection {
 	int state;
+
+	/* should be a new state */
+	int closed;
+
 	int rx_iostate;
 	int tx_iostate;
 	int refcount;
diff --git a/usr/tgtd.c b/usr/tgtd.c
index 758e7d5..1a3cc02 100644
--- a/usr/tgtd.c
+++ b/usr/tgtd.c
@@ -137,6 +137,7 @@ static struct event_data *tgt_event_lookup(int fd)
 void tgt_event_del(int fd)
 {
 	struct event_data *tev;
+	int ret;
 
 	tev = tgt_event_lookup(fd);
 	if (!tev) {
@@ -144,7 +145,10 @@ void tgt_event_del(int fd)
 		return;
 	}
 
-	epoll_ctl(ep_fd, EPOLL_CTL_DEL, fd, NULL);
+	ret = epoll_ctl(ep_fd, EPOLL_CTL_DEL, fd, NULL);
+	if (ret < 0)
+		eprintf("fail to remove epoll event, %s\n", strerror(errno));
+
 	list_del(&tev->e_list);
 	free(tev);
 }
-- 
1.5.6.5

--
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