[stgt] [PATCH 1/1] Allow setting number of threads from commandline

FUJITA Tomonori fujita.tomonori at lab.ntt.co.jp
Mon Sep 21 05:13:23 CEST 2009


On Mon, 21 Sep 2009 12:40:38 +1000
ronnie sahlberg <ronniesahlberg at gmail.com> wrote:

> From 4261a4704556029c7e1c134d29244e9a139b0bc1 Mon Sep 17 00:00:00 2001
> From: Ronnie Sahlberg <ronniesahlberg at gmail.com>
> Date: Mon, 21 Sep 2009 11:52:55 +1000
> Subject: [PATCH] Add an argument --threads/-t to allow setting the number of threads at runtime.
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg at gmail.com>
> ---
>  usr/bs.c        |   24 ++++++++++++++----------
>  usr/bs_mmap.c   |    2 +-
>  usr/bs_rdwr.c   |    2 +-
>  usr/bs_thread.h |    4 ++--
>  usr/tgtd.c      |   11 ++++++++++-
>  usr/tgtd.h      |    1 +
>  6 files changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/usr/bs.c b/usr/bs.c
> index d0fcce4..2b070c0 100644
> --- a/usr/bs.c
> +++ b/usr/bs.c
> @@ -276,6 +276,14 @@ int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn,
>  	pthread_mutex_init(&info->pending_lock, NULL);
>  	pthread_mutex_init(&info->startup_lock, NULL);
>  
> +	info->num_worker_threads = nr_threads;
> +	info->worker_threads = malloc(sizeof(pthread_t *) * nr_threads);
> +	if (info->worker_threads == NULL) {

Please do instead

if (!info->worker_threads) {


> +		eprintf("failed to allocate array for %d threads\n",
> +			nr_threads);
> +		goto destroy_cond_mutex;
> +	}
> +
>  	ret = pipe(info->command_fd);
>  	if (ret) {
>  		eprintf("failed to create command pipe, %m\n");
> @@ -303,14 +311,10 @@ int bs_thread_open(struct bs_thread_info *info, request_func_t *rfn,
>  		goto event_del;
>  	}
>  
> -	if (nr_threads > ARRAY_SIZE(info->worker_thread)) {
> -		eprintf("too many threads %d\n", nr_threads);
> -		nr_threads = ARRAY_SIZE(info->worker_thread);
> -	}
> -
> +	dprintf("starting %d threads\n", nr_threads);
>  	pthread_mutex_lock(&info->startup_lock);
>  	for (i = 0; i < nr_threads; i++) {
> -		ret = pthread_create(&info->worker_thread[i], NULL,
> +		ret = pthread_create(&info->worker_threads[i], NULL,
>  				     bs_thread_worker_fn, info);
>  
>  		if (ret) {
> @@ -339,7 +343,7 @@ destroy_threads:
>  
>  	pthread_mutex_unlock(&info->startup_lock);
>  	for (; i > 0; i--) {
> -		pthread_join(info->worker_thread[i - 1], NULL);
> +		pthread_join(info->worker_threads[i - 1], NULL);
>  		eprintf("stopped the worker thread %d\n", i - 1);
>  	}
>  event_del:
> @@ -372,9 +376,9 @@ void bs_thread_close(struct bs_thread_info *info)
>  	info->stop = 1;
>  	pthread_cond_broadcast(&info->pending_cond);
>  
> -	for (i = 0; info->worker_thread[i] &&
> -		     i < ARRAY_SIZE(info->worker_thread); i++)
> -		pthread_join(info->worker_thread[i], NULL);
> +	for (i = 0; info->worker_threads[i] &&
> +		     i < info->num_worker_threads; i++)
> +		pthread_join(info->worker_threads[i], NULL);

I think that you can do here:

> +	for (i = 0; i < info->num_worker_threads; i++)

Leaving it alone for safety is fine by me but you must use zalloc
instead of malloc for info->worker_threads.


>  	pthread_cond_destroy(&info->finished_cond);
>  	pthread_cond_destroy(&info->pending_cond);
> diff --git a/usr/bs_mmap.c b/usr/bs_mmap.c
> index b62c8e6..46c0981 100644
> --- a/usr/bs_mmap.c
> +++ b/usr/bs_mmap.c
> @@ -93,7 +93,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, NR_WORKER_THREADS);
> +	return bs_thread_open(info, bs_mmap_request, num_worker_threads);
>  }
>  
>  static void bs_mmap_exit(struct scsi_lu *lu)
> diff --git a/usr/bs_rdwr.c b/usr/bs_rdwr.c
> index 6068479..4dfc617 100644
> --- a/usr/bs_rdwr.c
> +++ b/usr/bs_rdwr.c
> @@ -146,7 +146,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, NR_WORKER_THREADS);
> +	return bs_thread_open(info, bs_rdwr_request, num_worker_threads);
>  }
>  
>  static void bs_rdwr_exit(struct scsi_lu *lu)
> diff --git a/usr/bs_thread.h b/usr/bs_thread.h
> index 9dfbbd5..0509059 100644
> --- a/usr/bs_thread.h
> +++ b/usr/bs_thread.h
> @@ -1,10 +1,10 @@
> -#define NR_WORKER_THREADS	4
>  
>  typedef void (request_func_t) (struct scsi_cmd *);
>  
>  struct bs_thread_info {
>  	pthread_t ack_thread;
> -	pthread_t worker_thread[NR_WORKER_THREADS];
> +	int num_worker_threads;
> +	pthread_t *worker_threads;
>  
>  	/* protected by pipe */
>  	struct list_head ack_list;
> diff --git a/usr/tgtd.c b/usr/tgtd.c
> index b07a445..fe5ae2b 100644
> --- a/usr/tgtd.c
> +++ b/usr/tgtd.c
> @@ -42,6 +42,7 @@
>  unsigned long pagesize, pageshift, pagemask;
>  
>  int system_active = 1;
> +int num_worker_threads;
>  static int ep_fd;
>  static char program_name[] = "tgtd";
>  static LIST_HEAD(tgt_events_list);
> @@ -50,12 +51,13 @@ static LIST_HEAD(tgt_sched_events_list);
>  static struct option const long_options[] =
>  {
>  	{"foreground", no_argument, 0, 'f'},
> +	{"threads", required_argument, 0, 't'},
>  	{"debug", required_argument, 0, 'd'},
>  	{"help", no_argument, 0, 'h'},
>  	{0, 0, 0, 0},
>  };
>  
> -static char *short_options = "fd:h";
> +static char *short_options = "fd:ht:";

keep them in order:

static char *short_options = "ft:d:h";



>  static void usage(int status)
>  {
> @@ -66,6 +68,7 @@ static void usage(int status)
>  		printf("\
>  Target framework daemon, version %s\n\
>    -f, --foreground        make the program run in the foreground\n\
> +  -t, --threads num       number of pthreads to use\n\
>    -d, --debug debuglevel  print debugging information\n\
>    -h, --help              display this help and exit\n\
>  ", TGT_VERSION);
> @@ -334,6 +337,9 @@ int main(int argc, char **argv)
>  		case 'f':
>  			is_daemon = 0;
>  			break;
> +		case 't':
> +			num_worker_threads = atoi(optarg);
> +			break;
>  		case 'd':
>  			is_debug = atoi(optarg);
>  			break;
> @@ -349,6 +355,9 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> +	if (num_worker_threads == 0)

if (!num_worker_threads)

> +		num_worker_threads = 4;

Please define something like NR_DEFAULT_WORKER_THREADS or something.


>  	err = log_init(program_name, LOG_SPACE_SIZE, is_daemon, is_debug);
>  	if (err)
>  		exit(1);
> diff --git a/usr/tgtd.h b/usr/tgtd.h
> index 303627e..fe8e4bb 100644
> --- a/usr/tgtd.h
> +++ b/usr/tgtd.h
> @@ -185,6 +185,7 @@ static inline int kreq_init(void)	\
>  #endif
>  
>  extern int system_active;
> +extern int num_worker_threads;
>  
>  extern int kspace_send_tsk_mgmt_res(struct mgmt_req *mreq);
>  extern int kspace_send_cmd_res(uint64_t nid, int result, struct scsi_cmd *);
> -- 
> 1.5.6
> 
--
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