Add IRQ placement-only and ithread-only API variants. intr_event_bind
has been extended with sibling methods, as it has many more callsites in
existing code.
Details
Details
- Reviewers
jhb kib mav adrian - Commits
- rS317756: Extend cpuset_get/setaffinity() APIs
(I moved the mutually unobjectionable part of D10435 to its own review.)
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
This is just the API and not any cpuset changes. Guess we can do the userland side of it later!
Comment Actions
Overall it is fine, see some trivial nits inline.
| sys/kern/kern_intr.c | ||
|---|---|---|
| 294 ↗ | (On Diff #27979) | Use bool for boolean arguments ? |
| 344 ↗ | (On Diff #27979) | This is racy beyond any tolerance. Two parallel setaffinity or one set and one get are not supposed to work correctly, it seems. I believe a low-tech but sufficient solution is to add global perhaps sleepable (sx) lock around both intr_setaffinity and intr_getaffinity. This is a subject for a separate patch, of course. |
| sys/sys/cpuset.h | ||
| 86 ↗ | (On Diff #27979) | I suggest CPU_WHICH_INTRHANDLER or similar name. IRQ_ONLY is puzzling. |
Comment Actions
Later, you would want to update cpuset(2).
| sys/kern/kern_intr.c | ||
|---|---|---|
| 328 ↗ | (On Diff #27986) | If you write this as if (error != 0 && bindithread) {you can de-indent the body by one level. |
| sys/kern/kern_intr.c | ||
|---|---|---|
| 328 ↗ | (On Diff #27986) | Sort of. The return (error) still needs to happen if error != 0 && !bindithread. So that would have to change into an } else if () {. And there's no compelling reason yet — none of the extra indented lines wrap. I'll leave it as-is for now. |
| sys/kern/kern_intr.c | ||
|---|---|---|
| 328 ↗ | (On Diff #27986) | If would be if (error != 0 && !bindithread) {
...
}
if (error != 0)
return (error);Ok. |