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

Fix wait_for_free_tx when ring is full #16

Merged
merged 1 commit into from Jan 15, 2015
Merged

Fix wait_for_free_tx when ring is full #16

merged 1 commit into from Jan 15, 2015

Conversation

@talex5
Copy link
Contributor

@talex5 talex5 commented Jan 9, 2015

Previously:

  1. We waited even if the ring had enough free space (if n >= numfree).
  2. We checked again as soon as the underlying event channel got an event. However, space doesn't become available in the ring until tx_poll processes the responses, which usually happens after that.

The effect of this is that large transfers would often hang or pause (pinging the unikernel would unstick it).

This change depends on mirage/shared-memory-ring#15

Previously:

1. We waited even if the ring had enough free space (n >= numfree)
2. We checked again as soon as the underlying event channel got an
   event. However, space doesn't become available in the ring until
   tx_poll processes the responses, which usually happens after that.
@avsm
Copy link
Member

@avsm avsm commented Jan 9, 2015

Looks correct to me.

@djs55
Copy link
Member

@djs55 djs55 commented Jan 9, 2015

Aha, good one. Good idea to add the waiting function to the ring code.

@djs55
Copy link
Member

@djs55 djs55 commented Jan 9, 2015

OK, the shared-memory-ring API is in 1.1.1 (I've added a temporary package to mirage/mirage-dev). Anyone object to merging this and releasing this too?

@avsm
Copy link
Member

@avsm avsm commented Jan 11, 2015

go ahead and release smr-1.1.1 to opam -- it's just an API addition so I see no harm in mainlining it.

@talex5
Copy link
Contributor Author

@talex5 talex5 commented Jan 15, 2015

Thanks @djs55 - so this fix can be merged now that smr-1.1.1 is out?

@djs55
Copy link
Member

@djs55 djs55 commented Jan 15, 2015

Let's do it!

djs55 added a commit that referenced this pull request Jan 15, 2015
Fix wait_for_free_tx when ring is full
@djs55 djs55 merged commit a598a32 into mirage:master Jan 15, 2015
1 check passed
1 check passed
@talex5
continuous-integration/travis-ci The Travis CI build passed
Details
@talex5 talex5 deleted the talex5:fix-wait branch Jan 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants