public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladislav Bolkhovitin <vst@vlnb.net>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: linux-kernel@vger.kernel.org
Subject: Dynamic switching of io_context
Date: Fri, 12 Dec 2008 22:18:39 +0300	[thread overview]
Message-ID: <4942B90F.6050807@vlnb.net> (raw)

[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]

Hello Jens,

In SCST (http://scst.sf.net) in some cases IO can be submitted 
asynchronously. This is possible for pass-through (i.e. using 
scsi_execute_async()) and BLOCKIO (i.e. using direct bio interface, see 
blockio_exec_rw() in 
http://scst.svn.sourceforge.net/viewvc/scst/trunk/scst/src/dev_handlers/scst_vdisk.c?revision=614&view=markup) 
backend. For them there's no need to have a per device pool of threads, 
one or more global thread(s) can perfectly do all the work. But it is 
very desirable for performance that all the IO is submitted in a 
dedicated IO context for each initiator (i.e. client), which originated 
it. I.e. commands from initiator 1 submitted in IO context IOC1, from 
initiator 2 - IOC2, etc. Most likely, the same approach would be very 
useful for NFS server as well.

To achieve that it is necessary to have a possibility to switch IO 
context of the threads on the fly. I tried to implement that (see the 
attached patch), but hit BUG_ON(!cic->dead_key) in cic_free_func(), when 
session for initiator with the corresponding IO context was being 
destroyed by scst_free_tgt_dev(). At that point it was guaranteed that 
there was no outstanding IO with this IO context.

So, I had to go to a more defensive approach to have for each pool of 
threads, including threads for async. IO, a dedicated IO context, which 
is currently implemented.

Could you advice please what was going wrong? What should I do to 
achieve what's desired?

Thanks,
Vlad

[-- Attachment #2: tgt_dev_io_context.diff --]
[-- Type: text/x-patch, Size: 2246 bytes --]

Index: scst/include/scst.h
===================================================================
--- scst/include/scst.h	(revision 583)
+++ scst/include/scst.h	(working copy)
@@ -1516,6 +1516,8 @@ struct scst_tgt_dev {
 	spinlock_t thr_data_lock;
 	struct list_head thr_data_list;
 
+	struct io_context *tgt_dev_ioc;
+
 	spinlock_t tgt_dev_lock;	/* per-session device lock */
 
 	/* List of UA's for this device, protected by tgt_dev_lock */
Index: scst/src/scst_lib.c
===================================================================
--- scst/src/scst_lib.c	(revision 583)
+++ scst/src/scst_lib.c	(working copy)
@@ -542,6 +542,12 @@ static struct scst_tgt_dev *scst_alloc_a
 	for (i = 0; i < (int)ARRAY_SIZE(tgt_dev->sn_slots); i++)
 		atomic_set(&tgt_dev->sn_slots[i], 0);
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 25)
+#if defined(CONFIG_BLOCK) && defined(SCST_ALLOC_IO_CONTEXT_EXPORTED)
+	tgt_dev->tgt_dev_ioc = alloc_io_context(GFP_KERNEL, -1);
+#endif
+#endif
+
 	if (dev->handler->parse_atomic &&
 	    (sess->tgt->tgtt->preprocessing_done == NULL)) {
 		if (sess->tgt->tgtt->rdy_to_xfer_atomic)
@@ -685,6 +691,8 @@ static void scst_free_tgt_dev(struct scs
 			scst_del_cmd_threads(vtt->threads_num);
 	}
 
+	put_io_context(tgt_dev->tgt_dev_ioc);
+
 	kmem_cache_free(scst_tgtd_cachep, tgt_dev);
 
 	TRACE_EXIT();
Index: scst/src/dev_handlers/scst_vdisk.c
===================================================================
--- scst/src/dev_handlers/scst_vdisk.c	(revision 583)
+++ scst/src/dev_handlers/scst_vdisk.c	(working copy)
@@ -751,6 +753,14 @@ static int vdisk_do_job(struct scst_cmd 
 			scst_thr_data_get(&thr->hdr);
 		} else
 			thr = container_of(d, struct scst_vdisk_thr, hdr);
+
+		EXTRACHECKS_WARN_ON(tsk->io_context);
+		/*
+		 * No need to call ioc_task_link(), because io_context will be
+		 * cleared in the end of this function.
+		 */
+		tsk->io_context = tgt_dev->tgt_dev_ioc;
+		TRACE_DBG_SPECIAL("io_context %p", tsk->io_context);
 	} else {
 		thr = &nullio_thr_data;
 		scst_thr_data_get(&thr->hdr);
@@ -1004,6 +1014,8 @@ out_done:
 	cmd->scst_cmd_done(cmd, SCST_CMD_STATE_DEFAULT, SCST_CONTEXT_SAME);
 
 out_thr:
+	tsk->io_context = NULL;
+
 	if (likely(thr != NULL))
 		scst_thr_data_put(&thr->hdr);
 

             reply	other threads:[~2008-12-12 19:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-12 19:18 Vladislav Bolkhovitin [this message]
     [not found] ` <20081216082235.GD3284@gandalf.sssup.it>
2008-12-17 18:52   ` Dynamic switching of io_context Vladislav Bolkhovitin
2008-12-18 13:40     ` Jens Axboe
2008-12-18 13:47       ` Vladislav Bolkhovitin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4942B90F.6050807@vlnb.net \
    --to=vst@vlnb.net \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox