[stgt] [PATCH] handle pthread_create failure properly in backing store pthread code

[FUJITA Tomonori]

On Fri, 26 Sep 2008 02:50:24 +0900
FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp> wrote:

> Seems the following patch works but I'm not sure I did the right
> thing.
> 
> =
> From: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
> Subject: [PATCH] handle pthread_create failure properly in backing store pthread code
> 
> This should fix the following bugs reported by Tomasz Chmielewski
> 
> - tgtd crash when more than 40 LUNs per target (and a way to reproduce)
> 
> http://lists.wpkg.org/pipermail/stgt/2008-September/002212.html
> 
> - tgtd crash when adding more than ~20 targets (and a way to reproduce)
> 
> http://lists.wpkg.org/pipermail/stgt/2008-September/002214.html
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
> ---
>  usr/bs.c     |   16 +++++++++++++++-
>  usr/target.c |    2 +-
>  2 files changed, 16 insertions(+), 2 deletions(-)

Oops, Tomasz told me that the patch can't be applied cleanly to the
latest git. I applied the following patch.

I'm not familiar with pthread programing. Please let me know if I do
something wrong.

Thanks,

diff --git a/usr/bs.c b/usr/bs.c
index 71b7767..cef7b19 100644
--- a/usr/bs.c
+++ b/usr/bs.c
@@ -148,7 +148,7 @@ static void *bs_thread_worker_fn(void *arg)
 			pthread_cond_wait(&info->pending_cond, &info->pending_lock);
 			if (info->stop) {
 				pthread_mutex_unlock(&info->pending_lock);
-				break;
+				pthread_exit(NULL);
 			}
 			goto retest;
 		}
@@ -208,8 +208,9 @@ int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn)
 	for (i = 0; i < ARRAY_SIZE(info->worker_thread); i++) {
 		ret = pthread_create(&info->worker_thread[i], NULL,
 				     bs_thread_worker_fn, info);
+		if (ret)
+			goto destroy_threads;
 	}
-
 rewrite:
 	ret = write(info->command_fd[1], &ret, sizeof(ret));
 	if (ret < 0) {
@@ -219,6 +220,20 @@ rewrite:
 	}
 
 	return 0;
+destroy_threads:
+	write(info->command_fd[1], &ret, sizeof(ret));
+	pthread_cancel(info->ack_thread);
+	pthread_cond_signal(&info->finished_cond);
+	pthread_join(info->ack_thread, NULL);
+
+	info->stop = 1;
+	for (i = 0; info->worker_thread[i]; i++) {
+		pthread_cancel(info->worker_thread[i]);
+		pthread_cond_signal(&info->pending_cond);
+	}
+
+	for (i = 0; info->worker_thread[i]; i++)
+		pthread_join(info->worker_thread[i], NULL);
 event_del:
 	tgt_event_del(info->done_fd[0]);
 close_done_fd:
diff --git a/usr/target.c b/usr/target.c
index 70bf72a..32812d9 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -491,7 +491,7 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params,
 	if (lu->bst->bs_init) {
 		ret = lu->bst->bs_init(lu);
 		if (ret)
-			goto fail_bs_init;
+			goto fail_lu_init;
 	}
 
 	if (backing && !path && !lu->attrs.removable) {
--
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