Testing drm-next Skylake suspend/resume I found problems with power management reference counting. After few suspend/resume cycles reference counter went negative. Investigation of that problem lead me to LinuxKPI workqueue wrappers, and in particular incorrect status reported by work cancel functions: they reported "successful" cancellation for works already completed in normal way. This patch fixes that plus some more edge cases around cancellation.
Details
Details
- Reviewers
• hselasky markj - Commits
- rS322272: Fix few issues of LinuxKPI workqueue.
This patch removes bunch of drm-next error messages during suspend/resume cycle.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Thank you for reviewing this code.
| sys/compat/linuxkpi/common/src/linux_work.c | ||
|---|---|---|
| 313 ↗ | (On Diff #31681) | This return value in the executing state does not match the comment above the function. Can you update the comment above the function? |
| 403 ↗ | (On Diff #31681) | This return value in the executing state does not match the comment above the function. Can you update the comment above the function? |
| sys/compat/linuxkpi/common/src/linux_work.c | ||
|---|---|---|
| 313 ↗ | (On Diff #31681) | I'd say it does match. In case of WORK_ST_EXEC work has already completed or at least started to execute, so this is not a cancellation from the caller point of view. It is merely a draining to guarantee there are not memory references left. |
| 403 ↗ | (On Diff #31681) | Same as above. |
| sys/compat/linuxkpi/common/src/linux_work.c | ||
|---|---|---|
| 483 ↗ | (On Diff #31681) | It's a good question, but I have no answer to it. This state is not obvious. If we managed to cancel callout or task, then this state is in fact idle. May be we could add another atomic to change WORK_ST_CANCEL to to WORK_ST_IDLE in that case and then indeed add it here? What do you think? |
| sys/compat/linuxkpi/common/src/linux_work.c | ||
|---|---|---|
| 483 ↗ | (On Diff #31681) | Pending in this case only means queued. The below one is do the other part. I guess that means WORK_ST_CANCEL is not a work queued state? |