sorry, above patch is wrong. I'll resend it later. On Fri, Oct 31, 2014 at 10:12 AM, Jinzhi Chen <nxtjin...@gmail.com> wrote:
> [PATCH] Initialize trace before journal init > > when journal thread init before the initialization of trace, > it enter into loop (using uninitailized `tid_max`)with > --debug-enable configured. we need to initialize trace before > journal thread init. > this patch extrace `wq_trace_init` from `init_work_queue` and > call it in the main() function just before the initialization > of journal file. > --- > lib/work.c | 48 ++++++++++++++++++++++-------------------------- > sheep/sheep.c | 9 +++++++++ > 2 files changed, 31 insertions(+), 26 deletions(-) > > diff --git a/lib/work.c b/lib/work.c > index 7e2cd79..e36516a 100644 > --- a/lib/work.c > +++ b/lib/work.c > @@ -70,6 +70,7 @@ static size_t nr_nodes = 1; > static size_t (*wq_get_nr_nodes)(void); > > static void *worker_routine(void *arg); > +int wq_trace_init (void); > > #ifdef HAVE_TRACE > > @@ -134,27 +135,6 @@ static void suspend(int num) > eventfd_xwrite(ack_efd, 1); /* ack of resume */ > } > > -static int wq_trace_init(void) > -{ > - tid_max = TID_MAX_DEFAULT; > - tid_map = alloc_bitmap(NULL, 0, tid_max); > - > - resume_efd = eventfd(0, EFD_SEMAPHORE); > - ack_efd = eventfd(0, EFD_SEMAPHORE); > - > - if (resume_efd < 0 || ack_efd < 0) { > - sd_err("failed to create event fds: %m"); > - return -1; > - } > - > - /* trace uses this signal to suspend the worker threads */ > - if (install_sighandler(SIGUSR2, suspend, false) < 0) { > - sd_debug("%m"); > - return -1; > - } > - return 0; > -} > - > static void trace_set_tid_map(int tid) > { > sd_mutex_lock(&tid_map_lock); > @@ -180,12 +160,32 @@ static void trace_clear_tid_map(int tid) > > #else > > -static inline int wq_trace_init(void) { return 0; } > static inline void trace_set_tid_map(int tid) {} > static inline void trace_clear_tid_map(int tid) {} > > #endif /* HAVE_TRACE */ > > +int wq_trace_init(void) > +{ > + tid_max = TID_MAX_DEFAULT; > + tid_map = alloc_bitmap(NULL, 0, tid_max); > + > + resume_efd = eventfd(0, EFD_SEMAPHORE); > + ack_efd = eventfd(0, EFD_SEMAPHORE); > + > + if (resume_efd < 0 || ack_efd < 0) { > + sd_err("failed to create event fds: %m"); > + return -1; > + } > + > + /* trace uses this signal to suspend the worker threads */ > + if (install_sighandler(SIGUSR2, suspend, false) < 0) { > + sd_debug("%m"); > + return -1; > + } > + return 0; > +} > + > static uint64_t get_msec_time(void) > { > struct timeval tv; > @@ -364,10 +364,6 @@ int init_work_queue(size_t (*get_nr_nodes)(void)) > return -1; > } > > - ret = wq_trace_init(); > - if (ret < 0) > - return ret; > - > ret = register_event(efd, worker_thread_request_done, NULL); > if (ret) { > sd_err("failed to register event fd %m"); > diff --git a/sheep/sheep.c b/sheep/sheep.c > index 2e91d0f..071e467 100644 > --- a/sheep/sheep.c > +++ b/sheep/sheep.c > @@ -148,6 +148,8 @@ static struct sd_option sheep_options[] = { > { 0, NULL, false, NULL }, > }; > > +extern int wq_trace_init (void); > + > static void usage(int status) > { > if (status) { > @@ -885,6 +887,13 @@ int main(int argc, char **argv) > goto cleanup_log; > } > > + /* We should init trace before journal init */ > + ret = wq_trace_init (); > + if (ret) { > + sd_err("failed to init trace"); > + goto cleanup_log; > + } > + > /* We should init journal file before backend init */ > if (uatomic_is_true(&sys->use_journal)) { > if (!strlen(jpath)) > -- > 1.7.9.5 > > > On Thu, Oct 30, 2014 at 4:14 PM, Hitoshi Mitake <mitake.hito...@gmail.com> > wrote: > >> On Thu, Oct 30, 2014 at 2:46 PM, Jinzhi Chen <nxtjin...@gmail.com> wrote: >> > thank you for your reply. >> > it sounds good to me. >> > >> > how about extract `wq_trace_init` from `init_work_queue` (lib/work.c) >> and >> > call it in the main() function just before the initialization of journal >> > file. >> > we can initialize work queue trace related variable before the >> > initialization of >> > work queue. In this manner, we don't need a new function. >> >> Seems good. Could you prepare v2 with the above scheme? >> >> Thanks, >> Hitoshi >> >> > >> > >> > Thanks >> > Jinzhi Chen >> > >> > On Thu, Oct 30, 2014 at 9:42 AM, Hitoshi Mitake >> > <mitake.hito...@lab.ntt.co.jp> wrote: >> >> >> >> >> >> Hi Jinzhi, thanks for your patch. >> >> >> >> At Wed, 29 Oct 2014 13:53:14 +0800, >> >> Jinzhi Chen wrote: >> >> > >> >> > [1 <multipart/alternative (7bit)>] >> >> > [1.1 <text/plain; UTF-8 (7bit)>] >> >> > journal thread uses `tid_max` which is not initialized until >> >> > later initial progress. The default value 0 cause journal >> >> > thread loop in the function `trace_set_tid_map` , so set >> >> > default value to 1. >> >> > >> >> > Signed-off-by: jinzhichen <nxtjin...@gmail.com> >> >> >> >> Could you write your name "Jinzhi Chen" instead of jinzhichen in >> >> Signed-off-by: line? >> >> >> >> > --- >> >> > lib/work.c | 2 +- >> >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> > >> >> > diff --git a/lib/work.c b/lib/work.c >> >> > index 7e2cd79..7b20685 100644 >> >> > --- a/lib/work.c >> >> > +++ b/lib/work.c >> >> > @@ -75,7 +75,7 @@ static void *worker_routine(void *arg); >> >> > >> >> > #define TID_MAX_DEFAULT 0x8000 /* default maximum tid for most >> systems >> >> > */ >> >> > >> >> > -static size_t tid_max; >> >> > +static size_t tid_max = 1; >> >> >> >> How about the below way? Extracting the code >> >> >> >> tid_max = TID_MAX_DEFAULT; >> >> tid_map = alloc_bitmap(NULL, 0, tid_max); >> >> >> >> from wq_trace_init(), prepare a new function, >> >> e.g. early_trace_init(), which does the above initialization, and let >> >> main() call the function in its early part. Practically your patch can >> >> solve the problem but the above way will not produce a combination of >> >> initialized tid_max and uninitialized tid_map. How do you think? >> >> >> >> Thanks, >> >> Hitoshi >> >> >> >> > static unsigned long *tid_map; >> >> > static struct sd_mutex tid_map_lock = SD_MUTEX_INITIALIZER; >> >> > >> >> > -- >> >> > 1.7.9.5 >> >> > [1.2 <text/html; UTF-8 (quoted-printable)>] >> >> > >> >> > [2 <text/plain; us-ascii (7bit)>] >> >> > -- >> >> > sheepdog mailing list >> >> > sheepdog@lists.wpkg.org >> >> > http://lists.wpkg.org/mailman/listinfo/sheepdog >> > >> > >> > >> > -- >> > sheepdog mailing list >> > sheepdog@lists.wpkg.org >> > http://lists.wpkg.org/mailman/listinfo/sheepdog >> > >> > >
-- sheepdog mailing list sheepdog@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/sheepdog