Messages in this thread | | | Date | Thu, 26 Jul 2012 12:47:48 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH 1/1] kthread: disable preemption during complete() |
| |
On Thu, 26 Jul 2012, Peter Zijlstra wrote: > On Wed, 2012-07-25 at 15:40 -0700, Tejun Heo wrote: > > > This patch disables preemption during complete(), since we call > > > schedule() directly afterwards, so it will correctly enter > > > TASK_UNINTERRUPTIBLE. This speeds up kthread creation/binding during > > > cpu hotplug significantly. > > tglx has patches that make the kthread create/destroy stuff from hotplug > go away.. that seems like the better approach.
Right. That cpu hotplug setup/teardown stuff is ugly.
> > > + > > > + /* > > > + * Disable preemption so we enter TASK_UNINTERRUPTIBLE after > > > + * complete() instead of possibly being preempted. This speeds > > > + * up clients that do a kthread_bind() directly after > > > + * creation. > > > + */ > > > + preempt_disable(); > > > > Shouldn't this happen before setting current state to UNINTERRUPTIBLE? > > What prevents preemption happening right above preempt_disable()? > > Nothing, it also doesn't matter that much, you could get preempted right > before preempt_disable() and end up in the same place. > > The main thing is avoiding the wakeup preemption from the complete() > because we're going to sleep right after anyway. > > The comment doesn't really make that clear.
Right, the comment is crap. It has nothing to do with kthread_bind() and stuff. The whole purpose is to avoid the pointless preemption after wakeup.
> > > complete(&create->done); > > > + preempt_enable_no_resched(); > > > + > > > schedule(); > > Other than that it seems fine, although I know tglx just loves new > preempt_enable_no_resched() sites ;-)
The ones which are immediately followed by a call to schedule() are at least not causing any headache for RT :)
Thanks,
tglx
| |