[Click] NotifierQueue on multi-threaded Click problem.

[Joonwoo Park]

Hi Eddie,
It's works perfectly.
My test machine has been working for 2 more days with sleepness 1.
Hurrah~~

Joonwoo Park

2007/9/14, Joonwoo Park <joonwpark81 at gmail.com>:
> And I I have started test with SLEEPINESS 1.
> Yep, I'll update the result in some days. (It fees good!)
>
> Joonwoo Park
>
> 2007/9/14, Eddie Kohler <kohler at cs.ucla.edu>:
> > Hooray!!!!!!!!!!
> > I will pretend that it is fixed (and expect to hear back from you in a couple
> > days ;) )
> >
> >
> > Joonwoo Park wrote:
> > > Certainly!
> > > (I tested it with SLEEPINESS 1)
> > >
> > > 2007/9/14, Eddie Kohler <kohler at cs.ucla.edu>:
> > >> Even with SLEEPINESS 1 ?  (SLEEPINESS 1 should work...)
> > >>
> > >>
> > >> Joonwoo Park wrote:
> > >>> You overpraise! :-)
> > >>> The move_thread() is working!, thanks.
> > >>> And I have started old test It looks prefect too.
> > >>> Thanks again.
> > >>>
> > >>> 2007/9/14, Eddie Kohler <kohler at cs.ucla.edu>:
> > >>>> Click is lucky to have you!
> > >>>>
> > >>>> There were a couple other cases of the same bug that your fix didn't catch; I
> > >>>> checked in a broader fix.  Does it work?
> > >>>>
> > >>>> E
> > >>>>
> > >>>>
> > >>>> Joonwoo Park wrote:
> > >>>>> Well..
> > >>>>> It's hard to agree with slowing down code.
> > >>>>> BTW Do I need something like this?
> > >>>>> Recently move_thread can make another thread's sleeping.
> > >>>>>
> > >>>>> diff --git a/lib/task.cc b/lib/task.cc
> > >>>>> index 197859c..a087b3b 100644
> > >>>>> --- a/lib/task.cc
> > >>>>> +++ b/lib/task.cc
> > >>>>> @@ -383,8 +383,8 @@ Task::move_thread(int thread_id)
> > >>>>>         if (old_thread->thread_id() != _home_thread_id
> > >>>>>             && old_thread->thread_id() != RouterThread::THREAD_STRONG_UNSCHEDULE
> > >>>>> ) {
> > >>>>>             if (scheduled()) {
> > >>>>> -               _should_be_scheduled = true;
> > >>>>>                 fast_unschedule();
> > >>>>> +               _should_be_scheduled = true;
> > >>>>>             }
> > >>>>>             _thread = _router->master()->thread(_home_thread_id);
> > >>>>>             old_thread->unlock_tasks();
> > >>>>>
> > >>>>> Joonwoo Park
> > >>>>>
> > >>>>> 2007/9/14, Eddie Kohler <kohler at cs.ucla.edu>:
> > >>>>>> AARGH!
> > >>>>>>
> > >>>>>> Well, thanks as usual for the bug report.
> > >>>>>>
> > >>>>>> I think though that this is one of those cases where your lock does not
> > >>>>>> actually fix the problem, it just makes the race condition much less
> > >>>>>> likely by slowing down the code.
> > >>>>>>
> > >>>>>> I have attempted to fix the problem in a different way.  The commit
> > >>>>>> message has more.  Let me know how it does....
> > >>>>>> Eddie
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> Joonwoo Park wrote:
> > >>>>>>> Unfortunately, A few hours ago I found that my test machine has gone
> > >>>>>>> into hibernation.
> > >>>>>>> It had worked for about 3 ~ 4 days maybe.
> > >>>>>>> And I found that I can reproduce the problem in short time with
> > >>>>>>> SLEEPINESS_TRIGGER=1.
> > >>>>>>> IMHO, I think process_pending() still has problem.
> > >>>>>>> When another thread's Task::add_pending see 'if (!_pending_nextptr)'
> > >>>>>>> as FALSE, at the same time if we set it to 0, thread->add_pending()
> > >>>>>>> cannot be called again (only if _pending_reschedule is FALSE).
> > >>>>>>> As a result, It's seems that a routerthread go sleep forever.
> > >>>>>>> I tried patch and It works fine although SLEEPINESS_TRIGGER=1. (I have
> > >>>>>>> never seen before working SLEEPINESS_TRIGGER=1)
> > >>>>>>> How about this?
> > >>>>>>>
> > >>>>>>> -
> > >>>>>>> Signed-off-by: Joonwoo Park <joonwpark81 at gmail.com>
> > >>>>>>>
> > >>>>>>>  lib/master.cc |    8 ++++++++
> > >>>>>>>  1 files changed, 8 insertions(+), 0 deletions(-)
> > >>>>>>>
> > >>>>>>> diff --git a/lib/master.cc b/lib/master.cc
> > >>>>>>> index 5a56371..cef62be 100644
> > >>>>>>> --- a/lib/master.cc
> > >>>>>>> +++ b/lib/master.cc
> > >>>>>>> @@ -370,7 +370,15 @@ Master::process_pending(RouterThread *thread)
> > >>>>>>>      // process the list
> > >>>>>>>      while (Task *t = Task::pending_to_task(my_pending)) {
> > >>>>>>>       my_pending = t->_pending_nextptr;
> > >>>>>>> +     // when another thread's Task::add_pending see 'if (!_pending_nextptr)'
> > >>>>>>> +     // as FALSE, at the same time if we set it to 0, thread->add_pending()
> > >>>>>>> +     // cannot be called again (only if _pending_reschedule is 0).
> > >>>>>>> +     bool lock = !t->_pending_reschedule;
> > >>>>>>> +     if (lock)
> > >>>>>>> +             flags = _task_lock.acquire();
> > >>>>>>>       t->_pending_nextptr = 0;
> > >>>>>>> +     if (lock)
> > >>>>>>> +             _task_lock.release(flags);
> > >>>>>>>       t->process_pending(thread);
> > >>>>>>>      }
> > >>>>>>>  }
> > >>>>>>> -
> > >>>>>>>
> > >>>>>>> Joonwoo Park
> >
>