Better testing when patching divers/staging/ - howto?

Better testing when patching divers/staging/ - howto?

[Matthias Beyer]

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-August/057437.html

-- 
Mit freundlichen Gr??en,
Kind regards,
Matthias Beyer

Proudly sent with mutt.
Happily signed with gnupg.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140905/1019fc30/attachment.bin 

Better testing when patching divers/staging/ - howto?

[Greg KH]

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

Better testing when patching divers/staging/ - howto?

[Valdis.Kletnieks at vt.edu]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140905/921680be/attachment.bin 

Better testing when patching divers/staging/ - howto?

[Mandeep Sandhu]

>
> 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

Better testing when patching divers/staging/ - howto?

[Matthias Beyer]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140905/db0f8311/attachment.bin 

Better testing when patching divers/staging/ - howto?

[Greg KH]

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

Better testing when patching divers/staging/ - howto?

[Mandeep Sandhu]

>
> 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

Better testing when patching divers/staging/ - howto?

[Greg KH]

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