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

Misc improvements #1245

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Misc improvements #1245

wants to merge 2 commits into from

Conversation

nunojsa
Copy link
Contributor

@nunojsa nunojsa commented Feb 14, 2025

PR Description

This improves iio_rwdev exit and is a combination of two things:

  1. When destroying the stream, make sure to dequeue any possible in flight block before destroying it. This makes for a much cleaner exit specially in cases where freeing the blocks is deferred to the default client which for some reason will make iiod choke for sometime on a io_submit() call related to in flight block dequeue;
  2. With 1), we don't really want to cancel the buffer as soon as we hit ctrl-c since then we would timeout in dequeing the block. And if we don't do 1), then we would still have the delay caused by iiod (since with Staging/iiod segfault #1243, iiod now makes sure it's safe to free a block before freeing it).

So this is really a combination of the above two points which improves things a lot when cancelling a buffer session with iio_rwdev.

Note this is only tested on linux so it would be great if someone could do a quick test on windows.

PR Type

  • Bug fix (a change that fixes an issue)
  • New feature (a change that adds new functionality)
  • Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • I have conducted a self-review of my own code changes
  • I have commented new code, particulary complex or unclear areas
  • I have checked that I did not intoduced new warnings or errors (CI output)
  • I have checked that components that use libiio did not get broken
  • I have updated the documentation accordingly (GitHub Pages, READMEs, etc)

@nunojsa
Copy link
Contributor Author

nunojsa commented Feb 17, 2025

Ups, I was testing this with other patches (part of open PRs)... But this should still work without them. I'll drop the unneeded commits.

Make sure to dequeue any possible in flight block before destroying it.
Note that if the block is not enqueued, this returns an error but we
really don't care about it. Making sure the block is dequeued makes
for a much cleaner exit specially in cases where freeing the blocks is
deferred to the default client.

Signed-off-by: Nuno Sá <[email protected]>
When destroying the stream, we'll try to dequeue (as it improves things
on iiod side) any possible pending block and if the buffer is already
cancelled we'll just timeout and make  exiting the app much slower. Hence,
let's try a clean exit first and if we get blocked a second hit on ctrl-c
will go and cancel the buffer as before.

Signed-off-by: Nuno Sá <[email protected]>
@nunojsa nunojsa force-pushed the staging/misc-improv branch from e29e5d3 to f615c07 Compare February 17, 2025 09:58
@dNechita
Copy link
Contributor

I have been running a "quick" test on Windows for a while now. However, I haven't been able to test your changes yet due to some issues with the MSVC 2019 build. The issue occurs both with my local build and with the artifacts from Azure and I am trying to figure out what is causing this.

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.

2 participants