[sheepdog] [PATCH] init trace for work queue before journal init

[Jinzhi Chen]

hi hitoshi,
I’m  so sorry that this patch introduce a bug in the dog command.
recently  I read the  source code of dog , it seems that 
dog uses `init_work_queue` as well as sheep! 
so extract `wq_trace_init` from `init_work_queue` causes 
dog’s new thread enter into  loop, because `tid_max` is not initialized.

all in all, the main problem is the code below will enter into loop when `tid_max` is 0.
---
		/* enlarge bitmap size */
                while (tid > tid_max)
                        tid_max *= 2;
—
and we need to init `tid_max` before journal init.
and we need to make sure both sheep and dog will not enter into loop.

maybe the solution is:
1. revert the patch & assign tid_max to 1 in its declaration 
or
2. call `wq_trace_init` in the dog’s main() function before `init_work_queue`
or
3. make sure `tid_max` is not 0 in `trace_set_tid_map`


what do you think


Thanks
Jinzhi Chen

> On Nov 13, 2014, at 10:07 AM, Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp> wrote:
> 
> At Wed, 12 Nov 2014 14:21:22 +0800,
> Jinzhi Chen wrote:
>> 
>> 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.
>> 
>> Signed-off-by: Jinzhi Chen <nxtjinzhi at gmail.com>
>> ---
>> include/work.h | 1 +
>> lib/work.c     | 8 ++------
>> sheep/sheep.c  | 7 +++++++
>> 3 files changed, 10 insertions(+), 6 deletions(-)
>> 
>> diff --git a/include/work.h b/include/work.h
>> index b2fa0cf..c2008ba 100644
>> --- a/include/work.h
>> +++ b/include/work.h
>> @@ -63,6 +63,7 @@ struct work_queue *create_work_queue(const char *name, enum wq_thread_control);
>> struct work_queue *create_ordered_work_queue(const char *name);
>> void queue_work(struct work_queue *q, struct work *work);
>> bool work_queue_empty(struct work_queue *q);
>> +int wq_trace_init(void);
>> 
>> #ifdef HAVE_TRACE
>> void suspend_worker_threads(void);
>> diff --git a/lib/work.c b/lib/work.c
>> index 7e2cd79..95953e4 100644
>> --- a/lib/work.c
>> +++ b/lib/work.c
>> @@ -134,7 +134,7 @@ static void suspend(int num)
>> 	eventfd_xwrite(ack_efd, 1); /* ack of resume */
>> }
>> 
>> -static int wq_trace_init(void)
>> +int wq_trace_init(void)
>> {
>> 	tid_max = TID_MAX_DEFAULT;
>> 	tid_map = alloc_bitmap(NULL, 0, tid_max);
>> @@ -180,7 +180,7 @@ static void trace_clear_tid_map(int tid)
>> 
>> #else
>> 
>> -static inline int wq_trace_init(void) { return 0; }
>> +inline int wq_trace_init(void) { return 0; }
> 
> Applied, thanks. But I removed the above inline because it is not
> required.
> 
> Thanks,
> Hitoshi
> 
> 
>> static inline void trace_set_tid_map(int tid) {}
>> static inline void trace_clear_tid_map(int tid) {}
>> 
>> @@ -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..ef45a33 100644
>> --- a/sheep/sheep.c
>> +++ b/sheep/sheep.c
>> @@ -885,6 +885,13 @@ int main(int argc, char **argv)
>> 		goto cleanup_log;
>> 	}
>> 
>> +	/* We should init trace for work queue before journal init */
>> +	ret = wq_trace_init();
>> +	if (ret) {
>> +		sd_err("failed to init trace for work queue");
>> +		goto cleanup_log;
>> +	}
>> +
>> 	/* We should init journal file before backend init */
>> 	if (uatomic_is_true(&sys->use_journal)) {
>> 		if (!strlen(jpath))
>> -- 
>> 1.9.3 (Apple Git-50)
>> 
>> -- 
>> sheepdog mailing list
>> sheepdog at lists.wpkg.org <mailto:sheepdog at lists.wpkg.org>
>> http://lists.wpkg.org/mailman/listinfo/sheepdog <http://lists.wpkg.org/mailman/listinfo/sheepdog>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wpkg.org/pipermail/sheepdog/attachments/20141124/0a2391f2/attachment-0004.html>