From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hitoshi Mitake <mitake.hitoshi@gmail.com>
Subject: Re: [PATCH v3] tgtd: refresh ready fds of event loop after event deletion
Date: Wed, 26 Feb 2014 14:33:40 +0900
Message-ID: <8738j64evf.wl%mitake.hitoshi@lab.ntt.co.jp>
References: <1393391602-6801-1-git-send-email-mitake.hitoshi@lab.ntt.co.jp>
	<20140226141945U.fujita.tomonori@lab.ntt.co.jp>
Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Return-path: <stgt-owner@vger.kernel.org>
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=20120113;
        h=date:message-id:from:to:cc:subject:in-reply-to:references
         :user-agent:mime-version:content-type;
        bh=1AcO8NH2fMCXqWCEG9BLnJe8lne18+LvRyaU/stNNTY=;
        b=dL/CzMsHA/qcjsaV9U/QE1wGOx5u8nYcFXv544wB9+ST1Q4rGrM3PqXLwUe23YEzzK
         4xSAYIeQQZZBdn5w8V6CS+a60CMYtsrLsgenMoRrMzifouH2x4NXlvs0+yjt8Zji5X0a
         qaxBmdaT14ECg599n2JgFmqdYO1iOXEAmZy6ikM1eFInDpOEFS0Qb3AKLtJHZyD4X9Eu
         VpKtDPRg9b4B3c8kUM3Ma1Y7kDi6OhnSzCtk2cfyJXUhkH1kszO61c3Hou+tW8CkijDr
         36lFwrCKpKGsBavqyqPkT9pX51FuOXhh4ZdiIi9wqVnOHHDV9bvWQa0IuyGs9E5uRbey
         IHQw==
In-Reply-To: <20140226141945U.fujita.tomonori@lab.ntt.co.jp>
Sender: stgt-owner@vger.kernel.org
List-ID: <stgt.vger.kernel.org>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: mitake.hitoshi@lab.ntt.co.jp, stgt@vger.kernel.org, mitake.hitoshi@gmail.com

At Wed, 26 Feb 2014 14:22:21 +0900,
FUJITA Tomonori wrote:
> 
> On Wed, 26 Feb 2014 14:13:22 +0900
> Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> wrote:
> 
> > Current main event loop (event_loop()) of tgtd has a possibility of
> > segmentation fault. The problem is caused by the below sequence:
> > 
> > 1. Event A, B are ready so epoll_wait(2) returns.
> > 2. The handler of the event A is called. In the event handler, the
> >    event B is deleted with tgt_event_del()
> > 3. event_loop() tries to call the handler of the event B. It causes
> >    segfault because the event struct is already removed and freed.
> > 
> > For avoid this problem, this patch adds a new global variable
> > event_need_refresh. If the value of this variable is 1, event_loop()
> > calls epoll_wait(2) again for refreshing ready fd list. This patch
> > also lets tgt_event_del() to turn on the flag in its tail.
> > 
> > For example, we can produce segfault of tgtd under heavy load. Below
> > is a backtrace obtained from the core file:
> >  (gdb) bt
> >  #0  0x0000000000000000 in ?? ()
> >  #1  0x0000000000411419 in event_loop () at tgtd.c:414
> >  #2  0x0000000000411b65 in main (argc=<value optimized out>, argv=<value optimized out>) at tgtd.c:591
> > 
> > To be honest, I still don't find an event handler which calls
> > tgt_event_del() for other fds. But with this modification, the above
> > segfault is avoided. The change seems to be effective.
> 
> As you pointed out off-line, making a connection closed via tgtadm
> might be the case. Anyway, I think that the handler API should allow
> removing other handlers from a handler. Merged, thanks a lot!
> 
> > +int event_need_refresh;
> > +
> 
> static?

Ah, the variable should be static one. I'll send a patch later.

Thanks,
Hitoshi