[stgt] help tgt segfault

[Jesse Nelson]

FYI i haven't had a segfault since upgrading to 0.9.3, and i have
defiantly had some slow network issues since then.

thanks fujita/tomasz !

On Tue, Dec 23, 2008 at 3:25 AM, FUJITA Tomonori
<fujita.tomonori at lab.ntt.co.jp> wrote:
> 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