[PATCH] kthread_{suspend, resume, suspend_check} locking bugs

[Jilles Tjoelker]

On Sun, Jan 10, 2010 at 02:09:43AM +0100, Attilio Rao wrote:
> I think that routines kthread_suspend(), kthread_resume() and
> kthread_suspend_check() are not adeguately SMP protected.
> That is because, in particular, the critical path doesn't protect,
> together, TDF_KTH_SUSP and sleeping activity. The right pattern should
> be to use the thread lock spinlock as an interlock and use msleep.
> Such bugs have not been revealed probabilly because there has been a
> lack of testing of such primitives and there are not, currently,
> consumers within our stock kernel.

> Additively, kthread_suspend_check() seems to require to always pass
> curthread, which is silly (as we don't have to conform to any
> particular KPI), thus I think it is appropriate for the prototype to
> change.
> The following patch should fix the issue:
> http://www.freebsd.org/~attilio/kthread.diff

The analysis and patch look sensible.

> That patch would have benefit from the priority argument (for PDROP)
> for msleep_spin(9), and I don't understand why we don't support it
> (the log message doesn't seem much clearer).
> If nobody objects, after this patch goes in I would add the priority
> argument to msleep_spin() too.

No objection here.

-- 
Jilles Tjoelker