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

ReservePolicies not independent from CreationPolicies #111

Open
Flamefire opened this issue Oct 14, 2015 · 4 comments
Open

ReservePolicies not independent from CreationPolicies #111

Flamefire opened this issue Oct 14, 2015 · 4 comments

Comments

@Flamefire
Copy link
Contributor

The CudaSetLimits policy could never be used with e.g. the Scatter creation policy which would lead to undefined behaviour. The reason is, that this ReservePolicy returns a NULL pointer but the creation policy expects a pointer to valid memory of the given size.

The Scatter policy should at least check, if the given pointer is non-NULL (which also helps #110) and it should be checked, if the creation and reserve policies should be merged and/or the CudaSetLimits policy removed to avoid these kind of pitfalls.

@slizzered
Copy link
Contributor

For the NULL-check: sounds very reasonable, PR coming soon.

For the discussion about the policies:
Unfortunately, not all policies are really compatible with each other. The problem is even more visible when dealing with the CUDA default allocator, since it doesn't expose/use things like a heap-pointer.

The solution might be a combination of extensive policy documentation and better constraints checking that enforces all the CUDA default allocator-specific policies to be used together. The constraints checking system itself is not really equipped to deal with groups of policies ("CudaSetLimits may not be used with any other CreationPolicy), so the temporary fix will be pretty ugly.

@Flamefire
Copy link
Contributor Author

Well what I mean: If the CudaSetLimits policy can only be used with the OldAlloc policy and the SimpleCudaMalloc can only be used with Scatter then those should be merged.
In the current codebase I can't find any other policies but I don't know if there are/could be more usefull combinations.

@ax3l ax3l added this to the 2.2.1crp: Bug Fixes milestone Dec 2, 2015
@ax3l ax3l added bug and removed bug labels Dec 2, 2015
@ax3l ax3l modified the milestones: 2.2.1crp: Bug Fixes, 2.3.0crp Dec 2, 2015
@ax3l
Copy link
Member

ax3l commented Dec 2, 2015

For the NULL-check: sounds very reasonable, PR coming soon.

can you please split the thread and open a new issue for the "bug"/safety feature aka NULL check? (for 2.2.1crp)

@Flamefire
Copy link
Contributor Author

Actually this cannot be split. The bug/safety feature is a work-around for the problem described here. The issue IS that the policies cannot be combined freely as suggested by the syntax.

However one can consider the missing null check in the scatter policy as a bug, so I'll open that as an issue: #115

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

No branches or pull requests

4 participants