From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hitoshi Mitake Subject: Re: [PATCH RFC v2] tgtd: send/recv iSCSI PDUs in worker threads Date: Sun, 24 Nov 2013 23:06:02 +0900 Message-ID: <8738mlzygl.wl%mitake.hitoshi@gmail.com> References: <1384073347-3067-1-git-send-email-mitake.hitoshi@gmail.com> <20131122090226H.fujita.tomonori@lab.ntt.co.jp> 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=oxCB1SZ80+VZphUR9bTkhFVPz55eMNm/Jdi1Uoy/5ic=; b=hWXV+Mtg7vEBiPJJGzOERyoYfJp6veDIOWEa/tPQeTvxvD3KJqR0F7ikPoaSGXvgTj jnjg4LgITKru58nNvDS7dHAj4y3LhnYtSKuDmwGT7/m3mkqFnE9s0wHX8e/HmB062aIq 9nCuy37HKAsf1AYVUJ6Xti4Ju7karIcFOpDemObWrJGRXMuext/DWmbYNXl+pvmv2Ctg n+Ub6X3MvjDVoL8bz724TNi+Bzupzk2eF9Vde/BYLCBNTpIFW/iCQYIAu9kCmh4mLC2n wbRh2zGAUlcUFpsxUB0cw5UYGkRdlHZxkXrwLgWZ2BznN13yRWgc1fEG6NtecArjcOR1 4Mhw== In-Reply-To: <20131122090226H.fujita.tomonori@lab.ntt.co.jp> 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 Fri, 22 Nov 2013 09:29:49 +0900, FUJITA Tomonori wrote: > > On Sun, 10 Nov 2013 17:49:07 +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 > > --- > > > > v2: > > - correct handling of connection closing based on a reference count of an iSCSI > > connection > > - a silly bug in iscsi_tcp_init() introduced in the previous patch is removed > > > > usr/iscsi/conn.c | 12 ++- > > usr/iscsi/iscsi_tcp.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++--- > > usr/iscsi/iscsid.c | 61 +++++++---- > > usr/iscsi/iscsid.h | 8 +- > > 4 files changed, 334 insertions(+), 40 deletions(-) > > This can handle a single iscsi session having multiple tcp > connections? For example, if iscsi_scsi_cmd_rx_start() is called by > multiple threads, conn->session->cmd_list is accessed by them without > a lock? As you say, some functions which manipulate data structures related to session can be called by multiple threads in an unsafe manner, sorry. I'll post v3 later. In the next version, iscsi_task_rx_start() and iscsi_task_tx_start() are executed in a serialized manner. AFAIK, other part of multithreaded code don't touch the data structures. Thanks for your review, Hitoshi