Without the patch, there is a race condition: when poll() is invoked(),
if kvp_globals.daemon_busy is false, the daemon won't be timely
woke up, because hv_kvp_send_msg_to_daemon() can't wake up the daemon
in this case.
Details
Details
- Reviewers
howard0su_gmail.com whu delphij royger - Group Reviewers
Contributor Reviewers (ports) - Commits
- rS292258: hyperv/kvp: wake up the daemon if it's sleeping due to poll()
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 1259 Build 1263: arc lint + arc unit
Event Timeline
Comment Actions
The patch has been in the status of Accepted for 6 days, but I didn't see it appear in the head of the svn repository.
Should I do something extra to make that happen?
This is my first patch for FreeBSD and the process is new to me. :-)
Comment Actions
AFAICT I think you are missing a:
seldrain(&hv_kvp_selinfo);
In hv_kvp_dev_close before setting kvp_globals.dev_accessed = false.
Also, now that I look at it, the hv_kvp_dev_open function seems to contain a race, you should use atomic_testandset_int in order to test and set kvp_globals.dev_accessed.
Comment Actions
Why? At most only 1 daemon is expected to open the dev file /dev/hv_kvp_dev. When the daemon is invoking close() and the execution enters the kernel funntion hv_kvp_dev_close(), we know the daemon is running, so I don't think we need to wake the daemon up?
Also, now that I look at it, the hv_kvp_dev_open function seems to contain a race, you should use atomic_testandset_int in order to test and set kvp_globals.dev_accessed.
Yes, I agree with you. I'll send a separate patch for this.
Comment Actions
Sorry for the delay, yesterday was a public holiday here. I agree that you make a special use of this device, so there's no risk in avoiding the seldrain in the close routine (because hv_kvp_selinfo is not going to be freed).
The same applies to hv_kvp_dev_destroy AFAICT, since you kill the thread and don't free the hv_kvp_selinfo struct there's no risk.
Roger.
Comment Actions
Thanks a lot for the explanation, Roger!
Since we have 2 'accept' s from delphij and you, I suppose the patch will be in head automatically. :-)
BTW, in future we'll carefully clean up the current hv_utils/kvp code , e.g., the race when accessing kvp_globals.dev_accessed.
Comment Actions
I'm not sure whether Wei will commit this or not. IMHO, we should solve this situation and clarify who is going to commit and maintain HyperV code. Let's see if Wei replies to the "Introducing new FreeBSD developers from Microsoft" and we can clarify the situation and decide how to move forward. If not I will commit it in a couple of days.