Better testing when patching divers/staging/ - howto?
Hi, I do big (cleanup) changes in drivers/staging/bcm/ Recently, I managed to piss off greg-kh by sending in a patch that broke the build[0]. After some time off, I want to re-start doing patches in this driver and of course do better! What I do by now: 1) Patching until my task is done. For example outsourcing stuff from functions in a file. 2) Before sending my patchset, compiling the appropriate patches like so: make drivers/staging/bcm/ 3) If it builds, generating patches, checking them and sending them. git format-patch gregkh-staging/staging-next..HEAD # more args scripts/checkpatch.pl ./00* git send-email # some args The error mentioned in [0] wasn't thrown when doing 2). My question to you: How to do better? How to test the patches even more? By building a whole kernel with them? Is that what greg-kh did and where the error he mentioned in [0] comes from? Or is there a test-suite-like thing around I just didn't discover yet? I do not have that much ressources to always build a full kernel for only one patchset, so would it be okay to (locally) merge my patchsets into a temporary branch and build a (allyesconfig) kernel out of all my patchsets? In addition, I do not have the appropriate hardware to actually _run_ the code. I always state this in my patchset messages, of course! I hope you guys can help me, as doing kernel patches is a nice hobby to me and I would like to continue. [0]: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2014-Augus... -- Mit freundlichen Grüßen, Kind regards, Matthias Beyer Proudly sent with mutt. Happily signed with gnupg.
On Fri, Sep 05, 2014 at 06:42:41PM +0200, Matthias Beyer wrote:
Hi,
I do big (cleanup) changes in
drivers/staging/bcm/
Recently, I managed to piss off greg-kh by sending in a patch that broke the build[0].
After some time off, I want to re-start doing patches in this driver and of course do better!
Try test-building your patches, that's a good start :)
What I do by now:
1) Patching until my task is done. For example outsourcing stuff from functions in a file.
2) Before sending my patchset, compiling the appropriate patches like so:
make drivers/staging/bcm/
Don't also forget a general 'make' for the whole tree, otherwise undefined symbols that you might have deleted from the module will not show up if you just build one subdirectory.
3) If it builds, generating patches, checking them and sending them.
git format-patch gregkh-staging/staging-next..HEAD # more args scripts/checkpatch.pl ./00* git send-email # some args
The error mentioned in [0] wasn't thrown when doing 2).
Because you only built the subdirectory...
My question to you: How to do better? How to test the patches even more? By building a whole kernel with them?
Yes.
Is that what greg-kh did and where the error he mentioned in [0] comes from?
Yes.
Or is there a test-suite-like thing around I just didn't discover yet?
Nope.
I do not have that much ressources to always build a full kernel for only one patchset, so would it be okay to (locally) merge my patchsets into a temporary branch and build a (allyesconfig) kernel out of all my patchsets?
Why? If you just change one file, 'make' will only rebuild that one file. Also do a faster make, with the -j option. Pass in 2x the number of CPU cores you have, so if you have 2 cores do: make -j4 to get a _much_ faster build.
In addition, I do not have the appropriate hardware to actually _run_ the code. I always state this in my patchset messages, of course!
It would be great if you could find some hardware, I really want to just delete this driver as no one seems to have the hardware anymore. You can only clean up just so much stuff without having to start to change the logic in the code, and you need the hardware to test that. hope this helps, greg k-h
On Fri, 05 Sep 2014 11:54:45 -0700, Greg KH said:
Don't also forget a general 'make' for the whole tree, otherwise undefined symbols that you might have deleted from the module will not show up if you just build one subdirectory.
If resources permit, do a 'make allnoconfig; make', a 'make allmodconfig; make', and a 'make allyesconfig; make' as well. A few randconfig's might not hurt too. This will often shake loose corner configuration cases where either a definition or a use is hiding inside a CONFIG_FOO in the source.
Also do a faster make, with the -j option. Pass in 2x the number of CPU cores you have, so if you have 2 cores do: make -j4
to get a _much_ faster build.
Isn't the usual "wisdom" to run as many jobs as the number of CPUs (like so grep processor /proc/cpuinfo | wc -l)? Wouldn't more of jobs simply contend for CPU time as there are more processes than processors? Thanks, -mandeep
On Fri, Sep 05, 2014 at 01:02:17PM -0700, Mandeep Sandhu wrote:
Also do a faster make, with the -j option. Pass in 2x the number of CPU cores you have, so if you have 2 cores do: make -j4
to get a _much_ faster build.
Isn't the usual "wisdom" to run as many jobs as the number of CPUs (like so grep processor /proc/cpuinfo | wc -l)?
I've always found 2x is "best". And I've been doing this for a while :)
Wouldn't more of jobs simply contend for CPU time as there are more processes than processors?
I/O is usually your limiting factor on kernel builds, not CPU time. You want to keep those CPUs "full" of stuff to do, so give them things to process while the i/o is completing from other tasks. But as always, try it out on your system to see what works best for you, maybe your system is CPU bound on kernel builds as you are running out of a tmpfs (hint, even then I've found that you want to have more jobs than the number of processors...) thanks, greg k-h
I/O is usually your limiting factor on kernel builds, not CPU time. You want to keep those CPUs "full" of stuff to do, so give them things to process while the i/o is completing from other tasks.
Yes, this makes sense as compilation is also quite I/O bound due to reading files to compile from disk.
But as always, try it out on your system to see what works best for you, maybe your system is CPU bound on kernel builds as you are running out of a tmpfs (hint, even then I've found that you want to have more jobs than the number of processors...)
I will. But thanks for the tip! :) -mandeep
thanks,
greg k-h
On 05-09-2014 11:54:45, Greg KH wrote:
I do not have that much ressources to always build a full kernel for only one patchset, so would it be okay to (locally) merge my patchsets into a temporary branch and build a (allyesconfig) kernel out of all my patchsets?
Why? If you just change one file, 'make' will only rebuild that one file.
Well, there comes git into play: If I do a build for each branch I have, this takes a huge amount of time for all patchsets, as when checking out another patchset, files get changed. At this very moment, I have 15 patchsets ready for submission and one I'm working on. Doing for ps in patchsets; do make -j 8; done still takes a lot of time then. That's why I asked the question.
Also do a faster make, with the -j option. Pass in 2x the number of CPU cores you have, so if you have 2 cores do: make -j4
to get a _much_ faster build.
Of course, I'm already doing this.
In addition, I do not have the appropriate hardware to actually _run_ the code. I always state this in my patchset messages, of course!
It would be great if you could find some hardware, I really want to just delete this driver as no one seems to have the hardware anymore. You can only clean up just so much stuff without having to start to change the logic in the code, and you need the hardware to test that.
This sounds like the work I'm doing is a waste of time? Shall I continue with my patches? If not, I will find another staging driver I can work on, so no problem! :-)
hope this helps,
Indeed it does. I did not expect to get an answer directly from you! Thank you a lot! -- Mit freundlichen Grüßen, Kind regards, Matthias Beyer Proudly sent with mutt. Happily signed with gnupg.
On Fri, Sep 05, 2014 at 10:11:19PM +0200, Matthias Beyer wrote:
On 05-09-2014 11:54:45, Greg KH wrote:
I do not have that much ressources to always build a full kernel for only one patchset, so would it be okay to (locally) merge my patchsets into a temporary branch and build a (allyesconfig) kernel out of all my patchsets?
Why? If you just change one file, 'make' will only rebuild that one file.
Well, there comes git into play: If I do a build for each branch I have, this takes a huge amount of time for all patchsets, as when checking out another patchset, files get changed.
At this very moment, I have 15 patchsets ready for submission and one I'm working on. Doing
for ps in patchsets; do make -j 8; done
still takes a lot of time then. That's why I asked the question.
What is a "patchset" in this context? You need to test the build at _every_ patch, to not do so isn't ok.
In addition, I do not have the appropriate hardware to actually _run_ the code. I always state this in my patchset messages, of course!
It would be great if you could find some hardware, I really want to just delete this driver as no one seems to have the hardware anymore. You can only clean up just so much stuff without having to start to change the logic in the code, and you need the hardware to test that.
This sounds like the work I'm doing is a waste of time? Shall I continue with my patches?
If not, I will find another staging driver I can work on, so no problem! :-)
I can't tell anyone what to work on, but again, I really want to just delete the driver as it is quite horrid. If you don't have the hardware for it, and you don't want to get it, I suggest working on something else that actually has a chance of getting merged to the proper portion of the kernel tree. No one who originally had the hardware does anymore, and wimax is all but dead, so it is not something anyone seems to care about. thanks, greg k-h
participants (4)
-
Greg KH -
Mandeep Sandhu -
Matthias Beyer -
Valdis.Kletnieks＠vt.edu