On Tue, Dec 17, 2013 at 11:42:53PM +0900, Hitoshi Mitake wrote:
> From: Hitoshi Mitake <[email protected]>
>
> The current build process of sheepdog compiles stuff for tracing even
> if tracing is disabled. Basically they are not harmful but causes
> memory consumption (tid_map), we should exlucde them when tracing is
> disabled.
>
> In addition, this patch adds a new mutex tid_map_lock for protecting
> tid_map. Previous work.c used wi->pending_lock also for protecting the
> bitmap. This protection scheme is very confusing.
>
> Signed-off-by: Hitoshi Mitake <[email protected]>
> ---
> include/work.h | 7 +++++--
> lib/Makefile.am | 4 ++++
> lib/work.c | 58
> ++++++++++++++++++++++++++++++++++++++++-------------
> sheep/Makefile.am | 1 +
> sheep/trace/trace.c | 1 +
> 5 files changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/include/work.h b/include/work.h
> index a5808b5..0cb3313 100644
> --- a/include/work.h
> +++ b/include/work.h
> @@ -61,9 +61,12 @@ static inline bool is_worker_thread(void)
> int init_work_queue(size_t (*get_nr_nodes)(void));
> struct work_queue *create_work_queue(const char *name, enum
> wq_thread_control);
> struct work_queue *create_ordered_work_queue(const char *name);
> -void suspend_worker_threads(void);
> -void resume_worker_threads(void);
> void queue_work(struct work_queue *q, struct work *work);
> bool work_queue_empty(struct work_queue *q);
>
> +#ifdef ENABLE_TRACE
> +void suspend_worker_threads(void);
> +void resume_worker_threads(void);
> +#endif /* BUILD_TRACE */
> +
> #endif
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index a681167..5204879 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -11,6 +11,10 @@ if BUILD_SHA1_HW
> libsheepdog_a_SOURCES += sha1_ssse3.S
> endif
>
> +if BUILD_TRACE
> +AM_CPPFLAGS += -DENABLE_TRACE
> +endif
> +
> # support for GNU Flymake
> check-syntax:
> $(COMPILE) -fsyntax-only $(CHK_SOURCES)
> diff --git a/lib/work.c b/lib/work.c
> index 6933e1a..84eb727 100644
> --- a/lib/work.c
> +++ b/lib/work.c
> @@ -33,13 +33,19 @@
> #include "work.h"
> #include "event.h"
>
> +#ifdef ENABLE_TRACE
> +
> #define TID_MAX_DEFAULT 0x8000 /* default maximum tid for most systems */
>
> static size_t tid_max;
> static unsigned long *tid_map;
> +static pthread_mutex_t tid_map_lock = PTHREAD_MUTEX_INITIALIZER;
> +
> static int resume_efd;
> static int ack_efd;
>
> +#endif /* ENABLE_TRACE */
> +
> /*
> * The protection period from shrinking work queue. This is necessary
> * to avoid many calls of pthread_create. Without it, threads are
> @@ -155,6 +161,8 @@ static int create_worker_threads(struct worker_info *wi,
> size_t nr_threads)
> return 0;
> }
>
> +#ifdef ENABLE_TRACE
> +
> void suspend_worker_threads(void)
> {
> struct worker_info *wi;
> @@ -197,6 +205,18 @@ void resume_worker_threads(void)
> }
> }
>
> +static void suspend(int num)
> +{
> + int uninitialized_var(value);
> +
> + eventfd_xwrite(ack_efd, 1); /* ack of suspend */
> + value = eventfd_xread(resume_efd);
> + assert(value == 1);
> + eventfd_xwrite(ack_efd, 1); /* ack of resume */
> +}
> +
> +#endif /* ENABLE_TRACE */
> +
> void queue_work(struct work_queue *q, struct work *work)
> {
> struct worker_info *wi = container_of(q, struct worker_info, q);
> @@ -252,7 +272,9 @@ static void *worker_routine(void *arg)
> /* started this thread */
> pthread_mutex_unlock(&wi->startup_lock);
>
> - pthread_mutex_lock(&wi->pending_lock);
> +#ifdef ENABLE_TRACE
> +
> + pthread_mutex_lock(&tid_map_lock);
> if (tid > tid_max) {
> size_t old_tid_max = tid_max;
>
> @@ -263,14 +285,22 @@ static void *worker_routine(void *arg)
> tid_map = alloc_bitmap(tid_map, old_tid_max, tid_max);
> }
> set_bit(tid, tid_map);
> - pthread_mutex_unlock(&wi->pending_lock);
> + pthread_mutex_unlock(&tid_map_lock);
> +
> +#endif /* ENABLE_TRACE */
How about grouping these lines into a function? This really messes up the code
and hard to maintain.
Then
#ifdef ENABLE_TRACE
functions() { do something }
#else
functions() { do nothing }
#endif
Thanks
Yuan
--
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog
