Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ex-lock utils setupVF finished #17

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add ex-lock utils setupVF finished #17

wants to merge 6 commits into from

Conversation

SchoIsles
Copy link

Sometimes, process allocated a free vf, but got error 'device not found' while setup it.
so, add a lock while find a free vf, other process will continue finding another free vf for use

@rkamudhan
Copy link
Collaborator

Hi @okletswin, Can you explain the problem in your setupVF ?

@hustcat
Copy link
Owner

hustcat commented Sep 25, 2017

fixup #16

sriov/sriov.go Outdated
@@ -29,6 +29,7 @@ func setupVF(conf *SriovConf, ifName string, netns ns.NetNS) error {
var (
err error
vfDevName string
locker *FileLocker
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about make locker as global variable and initialize it in init function?

sriov/sriov.go Outdated
if err != nil {
return err
}
} else {
// alloc a free virtual function
if vfIdx, vfDevName, err = allocFreeVF(masterName); err != nil {
if vfIdx, vfDevName, locker, err = allocFreeVF(masterName); err != nil {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe have no need to change allocFreeVF function, We can change it as follows:

locker.Lock()
allocFreeVF()
locker.UnLock()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, It's more better implementation. locker is global, Lock() maybe need an angument vfId and make it saved, Unlock() free the vfId and opened file. I will try

still make lock in getVFDeviceName
@SchoIsles
Copy link
Author

@hustcat , I pushed a new commit, and due to my level of golang is pitiful, some places are not too standard, improve it as you want

@hustcat
Copy link
Owner

hustcat commented Oct 18, 2017

@okletswin I made some changes, PTAL

Copy link
Author

@SchoIsles SchoIsles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, vf level none-block changed into process level with block, I think there is no problem

@SchoIsles SchoIsles closed this Oct 26, 2017
@SchoIsles SchoIsles reopened this Oct 26, 2017
@SchoIsles
Copy link
Author

@hustcat
I have tested, and fixed a piece of cake

@SchoIsles
Copy link
Author

哥,什么情况,咋么还没合并呢???

@hustcat
Copy link
Owner

hustcat commented Dec 1, 2017

@okletswin There are some conflicts, could you like to fix it?

Copy link
Author

@SchoIsles SchoIsles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retain setPF and setVF

@SchoIsles
Copy link
Author

@hustcat
pls merge

@houstar
Copy link

houstar commented Jan 27, 2021

@hustcat

IMO, in order to support high concurrency allocate VF, this file level lock is MUST.......

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants