Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 10 Jan 2010 02:09:43 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        freebsd-arch@freebsd.org
Subject:   [PATCH] kthread_{suspend, resume, suspend_check} locking bugs
Message-ID:  <3bbf2fe11001091709t228c48cft1c17af686e9e9c46@mail.gmail.com>

next in thread | raw e-mail | index | archive | help
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

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.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe11001091709t228c48cft1c17af686e9e9c46>