From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hitoshi Mitake Subject: Re: [PATCH RFC] tgtd: send/recv iSCSI PDUs in worker threads Date: Sun, 10 Nov 2013 17:39:07 +0900 Message-ID: <871u2ok5w4.wl%mitake.hitoshi@gmail.com> References: <1383541996-7946-1-git-send-email-mitake.hitoshi@gmail.com> <20131105.232845.1568838937885769584.fujita.tomonori@lab.ntt.co.jp> <87iow6ibmd.wl%mitake.hitoshi@lab.ntt.co.jp> <20131107154538Y.fujita.tomonori@lab.ntt.co.jp> <8738n4khfb.wl%mitake.hitoshi@gmail.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Return-path: 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=kKPNMBPl7JJLLH6J+Zt7MdI7g3Xzu7hGTPcMMA5BATU=; b=rc+7lP0PQzf+d4NJyoqbWkfVqylwBEIocQbMEwss+QwXktkJOGtW+yJvv665U4ngMl 3t0HvtZ/UnbC/yumMrrh4iNJtDxj3+6GVewHwut/OBAkojnfl5CvLrjNKHNeAzH16xbw PSUysIx2teddvZf+2VW0CViGfCT6MqteYp/zY1nKfoWQFqG2Wj3zmLgtMAO43si/7zyi WmvGMTOR9LSjGgWqDPfmXnCaBHdoKmIssPdbXjkmBAw7nF2XUGjcDgZnQWji7rtl+6iM fZcM5I1qA1N2wmprRvKA8N44PzlBCQX2pBafr8ugTkDQ+ShP+xnxlyrnm/UnWvUZqFNA D3MA== In-Reply-To: <8738n4khfb.wl%mitake.hitoshi@gmail.com> Sender: stgt-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: FUJITA Tomonori Cc: mitake.hitoshi@gmail.com, stgt@vger.kernel.org, mitake.hitoshi@lab.ntt.co.jp At Sun, 10 Nov 2013 13:30:00 +0900, Hitoshi Mitake wrote: > > At Thu, 7 Nov 2013 16:11:17 +0900, > FUJITA Tomonori wrote: > > > > On Wed, 06 Nov 2013 10:04:42 +0900 > > Hitoshi Mitake wrote: > > > > > At Tue, 05 Nov 2013 23:28:45 +0900 (JST), > > > FUJITA Tomonori wrote: > > > > > > > > On Mon, 4 Nov 2013 14:13:16 +0900 > > > > Hitoshi Mitake wrote: > > > > > > > > > From: Hitoshi Mitake > > > > > > > > > > Current tgtd sends and receives iSCSI PDUs in its main event > > > > > loop. This design can cause bottleneck when many iSCSI clients connect > > > > > to single tgtd process. For example, we need multiple tgtd processes > > > > > for utilizing fast network like 10 GbE because typical single > > > > > processor core isn't fast enough for processing bunch of requests. > > > > > > > > > > This patch lets tgtd send/receive iSCSI PDUs and check digests in its > > > > > worker threads. After applying this patch, the bottleneck in the main > > > > > event loop is removed and the performance is improved. > > > > > > > > > > The improvement can be seen even if tgtd and iSCSI initiator are > > > > > running on a single host. Below is a snippet of fio result on my > > > > > laptop. The workload is 128MB random RW. Backingstore is sheepdog. > > > > > > > > > > original tgtd: > > > > > read : io=65392KB, bw=4445.2KB/s, iops=1111, runt= 14711msec > > > > > write: io=65680KB, bw=4464.8KB/s, iops=1116, runt= 14711msec > > > > > > > > > > tgtd with this patch: > > > > > read : io=65392KB, bw=5098.9KB/s, iops=1274, runt= 12825msec > > > > > write: io=65680KB, bw=5121.3KB/s, iops=1280, runt= 12825msec > > > > > > > > > > This change will be more effective when a number of iSCSI clients > > > > > increases. I'd like to hear your comments on this change. > > > > > > > > > > Signed-off-by: Hitoshi Mitake > > > > > --- > > > > > usr/iscsi/iscsi_tcp.c | 291 +++++++++++++++++++++++++++++++++++++++++++++++--- > > > > > usr/iscsi/iscsid.c | 61 +++++++---- > > > > > usr/iscsi/iscsid.h | 4 + > > > > > 3 files changed, 322 insertions(+), 34 deletions(-) > > > > > > > > This change doesn't affect our complicated logic to handle outstanding > > > > commands with tcp disconnection (e.g. conn_close() in conn.c)? > > > > > > I think it doesn't affect the closing logic. Because any procedure > > > other than send/recv and digest checking are not delegated to worker > > > threads. Task queuing, connection closing, etc are done in main > > > thread even now. > > > > I've not read the patch but conn_close() has the code to handle a > > response to be being sent when a tcp connection is closed. > > Sorry, the above description is not correct. As you say, some > send/recv would be done in the main event loop > (e.g. iscsi_free_cmd_task()). But it wouldn't matter because > the logic of connection closing is preserved, and connection closing > isn't an event which affects performance of tgtd. On the second thought, this patch cannot handle the case when conn_close() doesn't release a connection. As you say, this would conflict with the closing logic. I'll fix it and send v2 later. Thanks, Hitoshi