On Tue, Oct 22, 2013 at 04:13:11PM +0900, Hitoshi Mitake wrote:
> 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.
> 
> Signed-off-by: Hitoshi Mitake <mitake.hito...@lab.ntt.co.jp>
> ---
>  include/work.h      |    7 +++++--
>  lib/Makefile.am     |    4 ++++
>  lib/work.c          |   50 ++++++++++++++++++++++++++++++++++++++------------
>  sheep/Makefile.am   |    1 +
>  sheep/trace/trace.c |    1 +
>  5 files changed, 49 insertions(+), 14 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 b6ac290..bab32b9 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 25cf964..d8e154e 100644
> --- a/lib/work.c
> +++ b/lib/work.c
> @@ -33,6 +33,8 @@
>  #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;
> @@ -40,6 +42,8 @@ static unsigned long *tid_map;
>  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 +159,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 +203,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,6 +270,8 @@ static void *worker_routine(void *arg)
>       /* started this thread */
>       pthread_mutex_unlock(&wi->startup_lock);
>  
> +#ifdef ENABLE_TRACE
> +
>       pthread_mutex_lock(&wi->pending_lock);
>       if (tid > tid_max) {
>               size_t old_tid_max = tid_max;
> @@ -265,12 +285,18 @@ static void *worker_routine(void *arg)
>       set_bit(tid, tid_map);
>       pthread_mutex_unlock(&wi->pending_lock);
>  
> +#endif       /* ENABLE_TRACE */
> +
>       while (true) {
>  
>               pthread_mutex_lock(&wi->pending_lock);
>               if (wq_need_shrink(wi)) {
>                       wi->nr_threads--;
> +
> +#ifdef ENABLE_TRACE
>                       clear_bit(tid, tid_map);
> +#endif

Instead of putting ENABLE_TRACE macros everywhere in the source, I'd suggest in
them together in following scheme:

#ifdef ENABLE_TRACE
functions()
{
         do something;
}
#else
functions() # whichi will be removed out by compiler automatically
{
        do nothing;
}

Thanks
Yuan
-- 
sheepdog mailing list
sheepdog@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/sheepdog

Reply via email to