[sheepdog] [PATCH 1/2] lib/work: provide wrappers of pthread for better logging

Hitoshi Mitake mitake.hitoshi at gmail.com
Tue Jul 29 02:44:10 CEST 2014


At Sun, 27 Jul 2014 21:47:28 +0900,
Hitoshi Mitake wrote:
> 
> From: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> 
> Currently some components of sheepdog uses pthread_create()
> directly. But this usage introduces diagnose unfriendly log. The below
> line is an example:
> 
> Jul 27 20:59:36   INFO [main] http_main_loop(325) http main loop
> 
> The function http_main_loop() is called in a worker thread but its
> prefix is [main]. Because The worker thread doesn't change its TLS
> variable worker_name (which is initialized by set_thread_name()). This
> patch adds a new wrapper sd_thread_create(), which wraps
> pthread_create() and let the worker threads set correct
> worker_name. Below line is a changed one:
> 
> Jul 27 21:33:27   INFO [http] http_main_loop(325) http main loop
> 
> This patch also introduces below wrappers:
>  - sd_thread_join(): a wrapper for pthread_join(), because pthread_t
>    is not used directly anymore
>  - sd_thread_create_with_idx(): another wrapper of pthread_create()
>    which produces several threads from single function
> 
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
>  include/work.h    |  7 +++++++
>  lib/work.c        | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  sheep/http/http.c |  5 +++--
>  sheep/md.c        | 13 +++++++------
>  sheep/nfs/nfsd.c  |  4 ++--
>  5 files changed, 76 insertions(+), 10 deletions(-)

Applied this series because nobody disagrees with it.

Thanks,
Hitoshi

> 
> diff --git a/include/work.h b/include/work.h
> index e26cb65..b2fa0cf 100644
> --- a/include/work.h
> +++ b/include/work.h
> @@ -69,4 +69,11 @@ void suspend_worker_threads(void);
>  void resume_worker_threads(void);
>  #endif	/* HAVE_TRACE */
>  
> +typedef pthread_t sd_thread_t;
> +int sd_thread_create(const char *, sd_thread_t *,
> +		     void *(*start_routine)(void *), void *);
> +int sd_thread_create_with_idx(const char *, sd_thread_t *,
> +		     void *(*start_routine)(void *), void *);
> +int sd_thread_join(sd_thread_t , void **);
> +
>  #endif
> diff --git a/lib/work.c b/lib/work.c
> index 35413ff..bb919a4 100644
> --- a/lib/work.c
> +++ b/lib/work.c
> @@ -444,3 +444,60 @@ bool work_queue_empty(struct work_queue *q)
>  
>  	return uatomic_read(&wi->nr_queued_work) == 0;
>  }
> +
> +struct thread_args {
> +	const char *name;
> +	void *(*start_routine)(void *);
> +	void *arg;
> +	bool show_idx;
> +};
> +
> +static void *thread_starter(void *arg)
> +{
> +	struct thread_args *args = (struct thread_args *)arg;
> +	void *ret;
> +
> +	set_thread_name(args->name, args->show_idx);
> +	ret = args->start_routine(args->arg);
> +	free(arg);
> +
> +	return ret;
> +}
> +
> +static int __sd_thread_create(const char *name, sd_thread_t *thread,
> +			      void *(*start_routine)(void *), void *arg,
> +			      bool show_idx)
> +{
> +	struct thread_args *args;
> +
> +
> +	args = xzalloc(sizeof(*args));
> +	args->name = name;
> +	args->start_routine = start_routine;
> +	args->arg = arg;
> +	args->show_idx = show_idx;
> +
> +	/*
> +	 * currently, there isn't a thread which uses ptread_attr_t
> +	 * in sheepdog
> +	 */
> +	return pthread_create(thread, NULL, thread_starter, args);
> +}
> +
> +int sd_thread_create(const char *name, sd_thread_t *thread,
> +		     void *(*start_routine)(void *), void *arg)
> +{
> +	return __sd_thread_create(name, thread, start_routine, arg, false);
> +}
> +
> +int sd_thread_create_with_idx(const char *name, sd_thread_t *thread,
> +			      void *(*start_routine)(void *), void *arg)
> +{
> +	return __sd_thread_create(name, thread, start_routine, arg, true);
> +}
> +
> +int sd_thread_join(sd_thread_t thread, void **retval)
> +{
> +	return pthread_join(thread, retval);
> +}
> +
> diff --git a/sheep/http/http.c b/sheep/http/http.c
> index 382937f..5786e90 100644
> --- a/sheep/http/http.c
> +++ b/sheep/http/http.c
> @@ -322,6 +322,7 @@ static void *http_main_loop(void *ignored)
>  {
>  	int err;
>  
> +	sd_info("http main loop");
>  	for (;;) {
>  		struct http_request *req = http_new_request(http_sockfd);
>  		int ret;
> @@ -414,7 +415,7 @@ static struct option_parser http_opt_parsers[] = {
>  
>  int http_init(const char *options)
>  {
> -	pthread_t t;
> +	sd_thread_t t;
>  	int err;
>  	char *s, address[HOST_NAME_MAX + 8];
>  
> @@ -446,7 +447,7 @@ int http_init(const char *options)
>  		return -1;
>  	}
>  	sd_info("http service listen at %s", address);
> -	err = pthread_create(&t, NULL, http_main_loop, NULL);
> +	err = sd_thread_create("http", &t, http_main_loop, NULL);
>  	if (err) {
>  		sd_err("%s", strerror(err));
>  		return -1;
> diff --git a/sheep/md.c b/sheep/md.c
> index d7724c0..8f4d5be 100644
> --- a/sheep/md.c
> +++ b/sheep/md.c
> @@ -413,7 +413,7 @@ main_fn int for_each_object_in_wd(int (*func)(uint64_t oid, const char *path,
>  	struct process_path_arg *thread_args, *path_arg;
>  	struct vnode_info *vinfo;
>  	void *ret_arg;
> -	pthread_t *thread_array;
> +	sd_thread_t *thread_array;
>  	int nr_thread = 0, idx = 0;
>  
>  	sd_read_lock(&md.lock);
> @@ -423,7 +423,7 @@ main_fn int for_each_object_in_wd(int (*func)(uint64_t oid, const char *path,
>  	}
>  
>  	thread_args = xmalloc(nr_thread * sizeof(struct process_path_arg));
> -	thread_array = xmalloc(nr_thread * sizeof(pthread_t));
> +	thread_array = xmalloc(nr_thread * sizeof(sd_thread_t));
>  
>  	vinfo = get_vnode_info();
>  
> @@ -434,9 +434,10 @@ main_fn int for_each_object_in_wd(int (*func)(uint64_t oid, const char *path,
>  		thread_args[idx].cleanup = cleanup;
>  		thread_args[idx].opaque = arg;
>  		thread_args[idx].result = SD_RES_SUCCESS;
> -		ret = pthread_create(thread_array + idx, NULL,
> -				     thread_process_path,
> -				     (void *)(thread_args + idx));
> +		ret = sd_thread_create_with_idx("foreach wd",
> +						thread_array + idx,
> +						thread_process_path,
> +						(void *)(thread_args + idx));
>  		if (ret) {
>  			/*
>  			 * If we can't create enough threads to process
> @@ -452,7 +453,7 @@ main_fn int for_each_object_in_wd(int (*func)(uint64_t oid, const char *path,
>  	sd_debug("Create %d threads for all path", nr_thread);
>  	/* wait for all threads to exit */
>  	for (idx = 0; idx < nr_thread; idx++) {
> -		ret = pthread_join(thread_array[idx], &ret_arg);
> +		ret = sd_thread_join(thread_array[idx], &ret_arg);
>  		if (ret)
>  			sd_err("Failed to join thread");
>  		if (ret_arg) {
> diff --git a/sheep/nfs/nfsd.c b/sheep/nfs/nfsd.c
> index 8ac8625..5cb87af 100644
> --- a/sheep/nfs/nfsd.c
> +++ b/sheep/nfs/nfsd.c
> @@ -198,10 +198,10 @@ uint64_t nfs_boot_time;
>  
>  int nfs_init(const char *options)
>  {
> -	pthread_t t;
> +	sd_thread_t t;
>  	int err;
>  
> -	err = pthread_create(&t, NULL, nfsd, NULL);
> +	err = sd_thread_create("nfs, "&t, nfsd, NULL);
>  	if (err) {
>  		sd_err("%s", strerror(err));
>  		return -1;
> -- 
> 1.9.1
> 



More information about the sheepdog mailing list