[sheepdog] [PATCH] lib/work: initialize `tid_max` to 1

Hitoshi Mitake mitake.hitoshi at gmail.com
Thu Oct 30 09:14:40 CET 2014


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
>



More information about the sheepdog mailing list