-
Notifications
You must be signed in to change notification settings - Fork 195
Fix glx reset #1315
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
base: master
Are you sure you want to change the base?
Fix glx reset #1315
Conversation
Turns out that glx does need this one. Assuming no external modules need this, it should be fine to keep this in a private header. Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
This commit broke glx after server reset. Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
|
Do we know what exactly was breaking/why these changes have to be reverted? |
glx is using the callback linked list in a weird way that is not compatible with the functions we already expose. |
|
Yeah, the initialisation |
I have not tested this theory, but I think that the old code was clearing the entire linked list except it's head, which was statically allocated, and the new code clears the entire list, which makes the rest of the glx code break. |
|
As a side note, I looked a bit info whether or not the X server leaks memory after each reset, and found that it leaks about 20 MB of memory for each reset (10GB for 500 resets). This is what I used to test it (with this patch applied): @metux Do you have any idea what might be leaking so much memory? |
|
I have idea (there are more very small leaks thus i wanted to collect more of them...), if using modesetting, mainly cursor leaking, try this one: if you have gcc with asan, you can add but it something 256kb per reset... |
Can you make a pr with this patch? |
I created #1324 |
how exactly did it break ? what are the symptoms ? |
symtom: on second With this patch it works correctly. |
|
Other symtom |
do you have some traffic dump ? which requests exactly are failing ? |
|
As i have asan working, adding it report with running note: this report after pressing ctrl-c on xserver, not every leak here have impact |
|
I’m not 100% sure, but my guess is that we shouldn’t delete the callback list completely there. We need to keep the 1st callback in the list. That’s because glxServer has At least that’s my working theory… |
On my setup, running |
|
As an alternative to this PR we could try re-adding the |
Can you make a pr? |
I can try, but I’m not sure when I will have time for that. So I am ok if anyone else does something like that, or even merging the current fix (reverting previous changes) and maybe returning to it later. |
These commit broke glx after server reset.