Re: [linux-usb-devel] init 1 kill khubd on 2.6.11

[Andrew Morton]

Nish Aravamudan <nish.aravamudan@xxxxxxxxx> wrote:
>
> > -       /* Send me a signal to get me die (for debugging) */
> >         do {
> >                 hub_events();
> > -               wait_event_interruptible(khubd_wait, !list_empty(&hub_event_list));
> > +               wait_event_interruptible(khubd_wait,
> > +                               !list_empty(&hub_event_list) ||
> > +                               kthread_should_stop());
> >                 try_to_freeze(PF_FREEZE);
> > -       } while (!signal_pending(current));
> > +       } while (!kthread_should_stop() || !list_empty(&hub_event_list));
> 
> Shouldn't this simply be a wait_event(), instead of
> wait_event_interruptible()?

That would cause uninterruptible sleep, which contributes to load average.

> Then the do-while() can be gotten rid of,
> as the only reason it is there currently, I guess, is to ignore
> signals?

Nope, the do-while is a basic part of the daemon's operation: keep doing
stuff until either there's no stuff to do or until we're told to exit.

> Also, the while's conditional should be (!kthread_should_stop() ||
> list_empty(&hub_event_list) to match the negation of wait_event's?
> (wait_event() expects the condition to stop on, while while() expects
> the condition to continue on)

Nope, the wait_event_interruptible test says

  "sleep unless the list is not empty or I am being asked to exit"

the while termination test says

  "loop until the list is empty and I am being asked to stop".

I think.  I had to scratch my head for a while over that code ;)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/