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 check for peer access. #311

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Conversation

devreal
Copy link
Contributor

@devreal devreal commented Nov 22, 2024

Yes, there was a bug. Yes, PaRSEC says devices cannot access themselves. This code should be trivial. Alas...

Yes, there was a bug. Yes, PaRSEC says devices cannot access themselves.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal devreal requested a review from abouteiller November 22, 2024 14:52
@abouteiller
Copy link
Contributor

I have been considering changing the mask in parsec to have self-access always be true, because that also helps with detecting if the peer-access masks are symmetrical.

@devreal
Copy link
Contributor Author

devreal commented Nov 22, 2024

Yes please. That is the only sane semantic. Ideally, I wouldn't even have to figure it out myself but just ask PaRSEC...

@abouteiller
Copy link
Contributor

That being said, why was having self-access causing an actual issue in the end? You should never issue D2D transfers from-to the same device anyways

@devreal devreal merged commit 334b83d into TESSEorg:master Nov 25, 2024
6 checks passed
@devreal
Copy link
Contributor Author

devreal commented Nov 25, 2024

That being said, why was having self-access causing an actual issue in the end? You should never issue D2D transfers from-to the same device anyways

Because I want to know whether all devices can access each other's memory. If they don't, we have to pushout to the host. Not having the self flag set makes the check more complex and bit me during tests.

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