Re: [PATCH 1/2]: workqueue: Implement the kernel API

[Krishna Kumar2]

Oleg Nesterov <oleg@xxxxxxxxxx> wrote on 09/22/2008 07:40:51 PM:

Thanks for your review comments, Oleg.

> > + * Passing delay=0 will result in immediate queueing of the entry,
whether
> > + * queue'd earlier or otherwise.
>
> The comment doesn't match the code ;)

Right! The delay=0 doesn't result in immediate queueing of the entry iff
the
current time has just hit the expiry time (or the timer has not fired yet).
The intention was to allow the existing code handle the timer/queueing in
this case, but I agree the comment has to be changed to explain that.

> > +       * Already present in workqueue. Check if the timer expiry is
> > +       * the same. Also, optimize in case requests are within one
> > +       * jiffy beyond the set expiry.
> > +       */
> > +      if (time_in_range(jiffies + delay, timer->expires,
> > +              timer->expires + 1))
> > +         return;
>
> Not that I argue, but do we really need to optimize this very unlikely
case?

Agree.

> This way the fast path is really fast, and the patch becomes simpler.
>
> What do you think? We can optimize the code (the usage of
cancel_work_sync)
> further, but perhaps this is enough.

Thanks for the suggestion. I will code it up and get some testing numbers.
Can I run
this by you?

Thanks,

- KK

--
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/