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

Install script bug #159

Open
lunatus opened this issue Sep 9, 2024 · 7 comments
Open

Install script bug #159

lunatus opened this issue Sep 9, 2024 · 7 comments

Comments

@lunatus
Copy link

lunatus commented Sep 9, 2024

This is a simple fix. I'm using a Raspberry Pi 4 with 1GB of ram and Ubuntu 24.04 for an embedded project. I was having problems with the baked in drivers so I built these. I discovered that the install script does not properly adjust for low memory systems. Line 288 should be make -j"${sproc}" not make -j"$(nproc)". I tweaked it and it now correctly uses less cores as specified earlier based on the amount of identified RAM. Four builds in parallel hosed my system. - Ben

@morrownr
Copy link
Owner

Hi @lunatus

Ben, sorry for the delay. I have looked at your post and was looking for docs on sproc but was not finding anything. Maybe I am loosing it. I've used nproc for a long time. I like good advice but I also don't like to break things for people.

Nick

@lunatus
Copy link
Author

lunatus commented Sep 27, 2024

Hi Nick, Sorry I was not clear. I'm more of a hardware guy than software so I've never actually pushed changes back to github and I didn't want to break anything either. Plus, these systems are sealed in a metal can and the onboard wifi is no longer able to connect to my network and these rtl wifi dongles are hosting a network on an external antenna so I can log into the system for debug.

"sproc" is a variable declared earlier up at line 161. It is then set based on the detected amount of available RAM. That happens properly and it informs the user at line 175 that so many out of n processors are going to be used. However, down at the build line 288, this is ignored and it launches nproc jobs in parallel. So, on say a Raspberry Pi 2,3,4 with 1GB RAM, the script fires off four parallel builds instead of two which will run the system out of memory and it halts. I just suggest fixing the make to use the variable and not the actual number of processors.

-Ben

@morrownr
Copy link
Owner

@lunatus

Ben,

Thanks so much for the explanation. Yes, it was a face-palm moment here. I have been busy on other projects since late last year so had not been looking at the code here except for API changes. I have now merged a patch so if you would be so kind as to take a look at the patch to make sure it is what you expected, I would appreciate it.

FYI: One of the projects I have been working on is improving the rtw88 in-kernel driver that supports the same chips as this driver. In fact, the work has greatly improved all supported rtw88 drivers and new rtl8821/11au and rtl8812au driver should be added over the next few weeks.

If there is any way for you to test your rtl8821/11cu adapters/modules with kernel 6.12 when it is available, probably Sunday. that would be great. If you find problems, I can connect you to the main developer that is working most of the code. Let me know.

@lunatus
Copy link
Author

lunatus commented Sep 27, 2024

Great! I may be able to test the script after next week and test with a new kernel. I'm not at a good point to test this on these systems as their first test deployment is Monday. I do have a spare pi and adapter I can put a cloned disk into and run rampant when I have time. These are some data collection buoys and I have them doing what I want right now. Today is not the day for foolery...

I did scan the change using a web browser and I think you might have actually made it worse. It needs the curly braces otherwise its going to try to execute sproc, not read it as a variable.

@lunatus
Copy link
Author

lunatus commented Sep 27, 2024 via email

@morrownr
Copy link
Owner

Ben,

I did scan the change using a web browser and I think you might have actually made it worse. It needs the curly braces otherwise its going to try to execute sproc, not read it as a variable.

Good catch. Patch has been merged.

Great! I may be able to test the script after next week and test with a new kernel.

I guess I was not clear, The testing I was talking about is for the rtw88 (in-kernel) driver, not the one in this repo. My intention is to archive this driver once the one in the kernel is in good enough shape.

Thanks.

@lunatus
Copy link
Author

lunatus commented Sep 27, 2024

Oh, I see. I'll let you know if I test it.

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

No branches or pull requests

2 participants