[stgt] [PATCH 1/2] DLL backing store
FUJITA Tomonori
fujita.tomonori at lab.ntt.co.jp
Fri Aug 22 01:58:28 CEST 2008
On Thu, 21 Aug 2008 13:34:01 -0700
"Richard Sharpe" <realrichardsharpe at gmail.com> wrote:
> On Thu, Aug 21, 2008 at 12:18 AM, FUJITA Tomonori
> <fujita.tomonori at lab.ntt.co.jp> wrote:
> > On Tue, 19 Aug 2008 14:52:23 +1000
> > Mark Harvey <markh794 at gmail.com> wrote:
> >
> >> Richard Sharpe wrote:
> >> > On Sun, Aug 17, 2008 at 10:48 PM, Mark Harvey <markh794 at gmail.com> wrote:
> >> >> From ea4d57fca07516c03980970cf90b937c45e3811e Mon Sep 17 00:00:00 2001
> >> >> From: Mark Harvey <markh794 at gmail.com>
> >> >> Date: Mon, 18 Aug 2008 15:03:21 +1000
> >> >> Subject: New backing store for ssc type devices.
> >> >>
> >> >> Uses a double-linked list header with each block of data.
> >> >>
> >> >> Implement basic fixed block READ_6 & WRITE_6 OP codes.
> >> >>
> >> >> Still along way to go.
> >> >> - Race condition on blk header between threads.
> >> >
> >> > Hmmm, why do you say there is a race here? Surely only one initiator
> >> > can access a tape at one time, and unless you have multiple threads
> >> > for the one tape, there should not be a problem here, unless I am
> >> > missing something?
> >> >
> >>
> >> It may be me that's missing something. I still have not got my head around the multi-threaded code.
> >>
> >> However I assume that 'multi-threaded' allows a worker thread to accept one SCSI op code.
> >>
> >> If the 2nd (or subsequent command) arrives during the update of the blk_header by the earlier thread, then there is a possibility of one thread using the blk_header belonging to the first thread and over writing each others data & header.
> >>
> >> e.g.
> >> writeA -> read blk_headerX (and start to copy/build new blk_headerY)
> >> writeB arrives before writeA has completed building blk_headerY
> >>
> >> Results is A & B writes will use the same blk_header (blk_headerX) information and over-write each others data.
> >>
> >> I am more then happy to be told I'm wrong.
> >
> > I've not looked at the code, but it sounds like you need to use
> > pthread_mutex_lock/unlock.
> >
> > Using only one thread could be a quick fix for now.
> >
> >
> >> But I see some simple locking process during the period of 'read header -> update new header & save in SSC header'. Then
> >> allow actual write/read of any data process to complete outside the mutex/spinlock
>
> Is there a simple mechanism to associate all CDBs for a device-LUN
> with a single thread?
We create four threads per lun now. It's pretty easy to simply
change the number of threads per lun (a patch is attached).
diff --git a/usr/bs.c b/usr/bs.c
index f100e2c..04ae647 100644
--- a/usr/bs.c
+++ b/usr/bs.c
@@ -165,7 +165,8 @@ static void *bs_thread_worker_fn(void *arg)
return NULL;
}
-int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn)
+int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn,
+ int nr_threads)
{
int i, ret;
@@ -197,11 +198,16 @@ int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn)
if (ret)
goto event_del;
- for (i = 0; i < ARRAY_SIZE(info->worker_thread); i++) {
+ if (nr_threads > ARRAY_SIZE(info->worker_thread))
+ nr_threads = ARRAY_SIZE(info->worker_thread);
+
+ for (i = 0; i < nr_threads; i++) {
ret = pthread_create(&info->worker_thread[i], NULL,
bs_thread_worker_fn, info);
}
+ info->nr_threads = nr_threads;
+
write(info->command_fd[1], &ret, sizeof(ret));
return 0;
@@ -232,7 +238,7 @@ void bs_thread_close(struct bs_thread_info *info)
info->stop = 1;
pthread_cond_broadcast(&info->pending_cond);
- for (i = 0; i < ARRAY_SIZE(info->worker_thread); i++)
+ for (i = 0; i < info->nr_threads; i++)
pthread_join(info->worker_thread[i], NULL);
pthread_cond_destroy(&info->finished_cond);
diff --git a/usr/bs_mmap.c b/usr/bs_mmap.c
index fff19d3..bb24f5e 100644
--- a/usr/bs_mmap.c
+++ b/usr/bs_mmap.c
@@ -96,7 +96,7 @@ static void bs_mmap_close(struct scsi_lu *lu)
static int bs_mmap_init(struct scsi_lu *lu)
{
struct bs_thread_info *info = BS_THREAD_I(lu);
- return bs_thread_open(info, bs_mmap_request);
+ return bs_thread_open(info, bs_mmap_request, NR_WORKER_THREADS);
}
static void bs_mmap_exit(struct scsi_lu *lu)
diff --git a/usr/bs_rdwr.c b/usr/bs_rdwr.c
index e2ece4a..b1fac8d 100644
--- a/usr/bs_rdwr.c
+++ b/usr/bs_rdwr.c
@@ -147,7 +147,7 @@ static int bs_rdwr_init(struct scsi_lu *lu)
{
struct bs_thread_info *info = BS_THREAD_I(lu);
- return bs_thread_open(info, bs_rdwr_request);
+ return bs_thread_open(info, bs_rdwr_request, 1);
}
static void bs_rdwr_exit(struct scsi_lu *lu)
diff --git a/usr/bs_ssc.c b/usr/bs_ssc.c
index dcc3e30..8affa95 100644
--- a/usr/bs_ssc.c
+++ b/usr/bs_ssc.c
@@ -208,7 +208,7 @@ static void bs_ssc_close(struct scsi_lu *lu)
static int bs_ssc_init(struct scsi_lu *lu)
{
struct bs_thread_info *info = BS_THREAD_I(lu);
- return bs_thread_open(info, ssc_rdwr_request);
+ return bs_thread_open(info, ssc_rdwr_request, NR_WORKER_THREADS);
}
static void bs_ssc_exit(struct scsi_lu *lu)
diff --git a/usr/bs_thread.h b/usr/bs_thread.h
index b97861c..75e5dc3 100644
--- a/usr/bs_thread.h
+++ b/usr/bs_thread.h
@@ -24,6 +24,7 @@ struct bs_thread_info {
int done_fd[2];
int stop;
+ int nr_threads;
request_func_t *request_fn;
};
@@ -33,7 +34,8 @@ static inline struct bs_thread_info *BS_THREAD_I(struct scsi_lu *lu)
return (struct bs_thread_info *) ((char *)lu + sizeof(*lu));
}
-extern int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn);
+extern int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn,
+ int nr_threads);
extern void bs_thread_close(struct bs_thread_info *info);
extern int bs_thread_cmd_submit(struct scsi_cmd *cmd);
--
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
More information about the stgt
mailing list