On Mon, 22 Dec 2008 21:52:26 +0100
Tomasz Chmielewski <mangoo@xxxxxxxx> 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@xxxxxxxxxxxxx>
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@xxxxxxxx> for helping me fix the
bugs.
Reported-by: Tomasz Chmielewski <mangoo@xxxxxxxx>
Tested-by: Tomasz Chmielewski <mangoo@xxxxxxxx>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
