[sheepdog] [PATCH] lib/work: initialize `tid_max` to 1
Jinzhi Chen
nxtjinzhi at gmail.com
Fri Oct 31 03:12:46 CET 2014
[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.hitoshi at gmail.com>
wrote:
> On Thu, Oct 30, 2014 at 2:46 PM, Jinzhi Chen <nxtjinzhi at 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.hitoshi at 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 <nxtjinzhi at 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 at lists.wpkg.org
> >> > http://lists.wpkg.org/mailman/listinfo/sheepdog
> >
> >
> >
> > --
> > sheepdog mailing list
> > sheepdog at lists.wpkg.org
> > http://lists.wpkg.org/mailman/listinfo/sheepdog
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wpkg.org/pipermail/sheepdog/attachments/20141031/83525cbc/attachment-0004.html>
More information about the sheepdog
mailing list