2018-09-28 02:09:17

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 0/7] Remove errors building drivers/DRIVERNAME

Special thanks for the feedback from:
- Finn Thain (I fixed the build problem)
- Geert Uytterhoeven (The cross compilers were very useful)
- Rolf Eike Beer (Was unintentional, thanks for the help!)

This Patchset changes some driver's Makefile to allow them building
using the command 'make drivers/DRIVERNAME', if compatible.

The changed drivers would return error if the above command was run
on them, after an x86 allyesconfig.

The main reason of this patchset is to allow building lists of
drivers looking for warnings and errors to be fixed.

I see this change as a new feature, not a bugfix. I understand
the default bahavior may be building with a simple 'make', but I
believe adding this new possibility will not be harmful.

My main objective is to allow developers with low processing power
to do changes in the kernel and look bugs using free services like
GiltabCI, before submitting to community.

If there is any interest helping/using this, I have a prototype in:
https://gitlab.com/LeoBras/linux-next


Leonardo Brás (7):
drivers: dio: Avoids building driver if CONFIG_DIO is disabled
drivers: nubus: Avoids building driver if CONFIG_NUBUS is disabled
drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled
drivers: zorro: Avoids building proc.o if CONFIG_ZORRO is disabled
drivers: s390: Avoids building drivers if ARCH is not s390.
drivers: oprofile: Avoids building driver from direct make command
drivers: hwtracing: Adds Makefile to enable building from directory.

drivers/Makefile | 4 +---
drivers/dio/Makefile | 2 +-
drivers/hwtracing/Makefile | 3 +++
drivers/nubus/Makefile | 5 +++--
drivers/oprofile/Makefile | 1 +
drivers/parisc/Makefile | 2 +-
drivers/s390/Makefile | 8 ++++----
drivers/zorro/Makefile | 7 ++++---
8 files changed, 18 insertions(+), 14 deletions(-)
create mode 100644 drivers/hwtracing/Makefile
create mode 100644 drivers/oprofile/Makefile

--
2.19.0



2018-09-28 02:09:10

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 1/7] drivers: dio: Avoids building driver if CONFIG_DIO is disabled

Avoids building driver if 'make drivers/dio/' is called and
CONFIG_DIO is disabled.

Signed-off-by: Leonardo Brás <[email protected]>
---
drivers/dio/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dio/Makefile b/drivers/dio/Makefile
index ae92d17083f2..8fc80e805b90 100644
--- a/drivers/dio/Makefile
+++ b/drivers/dio/Makefile
@@ -2,4 +2,4 @@
# Makefile for the linux kernel.
#

-obj-y := dio.o dio-driver.o dio-sysfs.o
+obj-$(CONFIG_DIO) := dio.o dio-driver.o dio-sysfs.o
--
2.19.0


2018-09-28 02:09:32

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

Avoids building driver if 'make drivers/parisc/' is called and
CONFIG_PARISC is disabled.

Signed-off-by: Leonardo Brás <[email protected]>
---
drivers/parisc/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
index 3cd5e6cb8478..80049d763aa0 100644
--- a/drivers/parisc/Makefile
+++ b/drivers/parisc/Makefile
@@ -24,5 +24,5 @@ obj-$(CONFIG_EISA) += eisa.o eisa_enumerator.o eisa_eeprom.o
obj-$(CONFIG_SUPERIO) += superio.o
obj-$(CONFIG_CHASSIS_LCD_LED) += led.o
obj-$(CONFIG_PDC_STABLE) += pdc_stable.o
-obj-y += power.o
+obj-$(CONFIG_PARISC) += power.o

--
2.19.0


2018-09-28 02:09:41

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 4/7] drivers: zorro: Avoids building proc.o if CONFIG_ZORRO is disabled

Avoids building proc.o if 'make drivers/zorro/' is called and
CONFIG_ZORRO is disabled, even if CONFIG_PROC_FS is enabled.

Signed-off-by: Leonardo Brás <[email protected]>
---
drivers/zorro/Makefile | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/zorro/Makefile b/drivers/zorro/Makefile
index b360ac4ea846..d580f9f08e0a 100644
--- a/drivers/zorro/Makefile
+++ b/drivers/zorro/Makefile
@@ -3,9 +3,10 @@
# Makefile for the Zorro bus specific drivers.
#

-obj-$(CONFIG_ZORRO) += zorro.o zorro-driver.o zorro-sysfs.o
-obj-$(CONFIG_PROC_FS) += proc.o
-obj-$(CONFIG_ZORRO_NAMES) += names.o
+obj-$(CONFIG_ZORRO) := zorro_all.o
+zorro_all-y += zorro.o zorro-driver.o zorro-sysfs.o
+zorro_all-$(CONFIG_ZORRO_NAMES) += names.o
+zorro_all-$(CONFIG_PROC_FS) += proc.o

hostprogs-y := gen-devlist

--
2.19.0


2018-09-28 02:10:09

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 5/7] drivers: s390: Avoids building drivers if ARCH is not s390.

Avoids building s390 drivers if 'make drivers/s390/' is called but
ARCH is not s390.

Signed-off-by: Leonardo Brás <[email protected]>
---
drivers/s390/Makefile | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/s390/Makefile b/drivers/s390/Makefile
index a863b0462b43..0575f02dba45 100644
--- a/drivers/s390/Makefile
+++ b/drivers/s390/Makefile
@@ -3,7 +3,7 @@
# Makefile for the S/390 specific device drivers
#

-obj-y += cio/ block/ char/ crypto/ net/ scsi/ virtio/
-
-drivers-y += drivers/s390/built-in.a
-
+ifeq ($(ARCH),s390)
+ obj-y += cio/ block/ char/ crypto/ net/ scsi/ virtio/
+ drivers-y += drivers/s390/built-in.a
+endif
--
2.19.0


2018-09-28 02:10:17

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 6/7] drivers: oprofile: Avoids building driver from direct make command

Creates new Makefile to avoid building driver if
'make drivers/oprofile/' is called directly.

This driver is usually built from arch/$ARCH and seems to have
no meaning building alone.

Signed-off-by: Leonardo Brás <[email protected]>
---
drivers/oprofile/Makefile | 1 +
1 file changed, 1 insertion(+)
create mode 100644 drivers/oprofile/Makefile

diff --git a/drivers/oprofile/Makefile b/drivers/oprofile/Makefile
new file mode 100644
index 000000000000..361867ec2338
--- /dev/null
+++ b/drivers/oprofile/Makefile
@@ -0,0 +1 @@
+#Does nothing, since the source is called from arch/$ARCH/ tree.
--
2.19.0


2018-09-28 02:10:33

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 2/7] drivers: nubus: Avoids building driver if CONFIG_NUBUS is disabled

Avoids building driver if 'make drivers/nubus/' is called and
CONFIG_NUBUS is disabled.
Avoids building proc.o if CONFIG_PROC_FS is enabled but
CONFIG_NUBUS is disabled.

Signed-off-by: Leonardo Brás <[email protected]>
---
drivers/nubus/Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nubus/Makefile b/drivers/nubus/Makefile
index 6d063cde39d1..1daa51217e95 100644
--- a/drivers/nubus/Makefile
+++ b/drivers/nubus/Makefile
@@ -2,6 +2,7 @@
# Makefile for the nubus specific drivers.
#

-obj-y := nubus.o bus.o
+obj-$(CONFIG_NUBUS) := nubus_all.o
+nubus_all-y += bus.o nubus.o

-obj-$(CONFIG_PROC_FS) += proc.o
+nubus_all-$(CONFIG_PROC_FS) += proc.o
--
2.19.0


2018-09-28 02:11:38

by Leonardo Brás

[permalink] [raw]
Subject: [PATCH v3 7/7] drivers: hwtracing: Adds Makefile to enable building from directory.

Adds Makefile to enable building the driver using
'make drivers/hwtracing/'.
Changes drivers/Makefile to call the new Makefile directly.
It enables user building this driver without building the whole drivers/
subtree.

Signed-off-by: Leonardo Brás <[email protected]>
---
drivers/Makefile | 4 +---
drivers/hwtracing/Makefile | 3 +++
2 files changed, 4 insertions(+), 3 deletions(-)
create mode 100644 drivers/hwtracing/Makefile

diff --git a/drivers/Makefile b/drivers/Makefile
index 578f469f72fb..a237be6b602f 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -174,9 +174,7 @@ obj-$(CONFIG_MCB) += mcb/
obj-$(CONFIG_PERF_EVENTS) += perf/
obj-$(CONFIG_RAS) += ras/
obj-$(CONFIG_THUNDERBOLT) += thunderbolt/
-obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
-obj-y += hwtracing/intel_th/
-obj-$(CONFIG_STM) += hwtracing/stm/
+obj-y += hwtracing/
obj-$(CONFIG_ANDROID) += android/
obj-$(CONFIG_NVMEM) += nvmem/
obj-$(CONFIG_FPGA) += fpga/
diff --git a/drivers/hwtracing/Makefile b/drivers/hwtracing/Makefile
new file mode 100644
index 000000000000..fe5773caec49
--- /dev/null
+++ b/drivers/hwtracing/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_CORESIGHT) += coresight/
+obj-y += intel_th/
+obj-$(CONFIG_STM) += stm/
--
2.19.0


2018-09-28 07:16:51

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

On Thu, 2018-09-27 at 23:08 -0300, Leonardo Brás wrote:
> Avoids building driver if 'make drivers/parisc/' is called and
> CONFIG_PARISC is disabled.

Is that really a problem? The drivers/Makefile has this:

obj-$(CONFIG_PARISC) += parisc/ 
And you just overrode that by forcing the build. It's not even clear
we should refuse the build in that case; how would we know you don't
have a legitimate reason for the override?

Signed-off-by: Leonardo Brás <[email protected]>
> ---
>  drivers/parisc/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
> index 3cd5e6cb8478..80049d763aa0 100644
> --- a/drivers/parisc/Makefile
> +++ b/drivers/parisc/Makefile
> @@ -24,5 +24,5 @@ obj-$(CONFIG_EISA) += eisa.o
> eisa_enumerator.o eisa_eeprom.o
>  obj-$(CONFIG_SUPERIO) += superio.o
>  obj-$(CONFIG_CHASSIS_LCD_LED) += led.o
>  obj-$(CONFIG_PDC_STABLE) += pdc_stable.o
> -obj-y += power.o
> +obj-$(CONFIG_PARISC) += power.o

If we conclude the use case is legitimate, that's not enough: the two
inner symbols are PARISC only but CONFIG_EISA isn't.

James


2018-10-01 07:56:46

by Robert Richter

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] Remove errors building drivers/DRIVERNAME

On 27.09.18 23:08:09, Leonardo Br?s wrote:
> This Patchset changes some driver's Makefile to allow them building
> using the command 'make drivers/DRIVERNAME', if compatible.
>
> The changed drivers would return error if the above command was run
> on them, after an x86 allyesconfig.

I don't see what you are trying to achieve here. Why shouldn't the
command fail if it is not the intended way to call it? There are a
couple of use cases where drivers/ is used to share common code over
different archs and it is not always the intention to build them in
drivers/ directly.

>
> The main reason of this patchset is to allow building lists of
> drivers looking for warnings and errors to be fixed.

If a list is the intention here, aren't there other ways to create it
other than using drivers/*?

-Robert

>
> I see this change as a new feature, not a bugfix. I understand
> the default bahavior may be building with a simple 'make', but I
> believe adding this new possibility will not be harmful.
>
> My main objective is to allow developers with low processing power
> to do changes in the kernel and look bugs using free services like
> GiltabCI, before submitting to community.
>
> If there is any interest helping/using this, I have a prototype in:
> https://gitlab.com/LeoBras/linux-next

2018-10-01 12:46:56

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] drivers: s390: Avoids building drivers if ARCH is not s390.

On Thu, Sep 27, 2018 at 11:08:14PM -0300, Leonardo Br?s wrote:
> Avoids building s390 drivers if 'make drivers/s390/' is called but
> ARCH is not s390.
>
> Signed-off-by: Leonardo Br?s <[email protected]>
> ---
> drivers/s390/Makefile | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/s390/Makefile b/drivers/s390/Makefile
> index a863b0462b43..0575f02dba45 100644
> --- a/drivers/s390/Makefile
> +++ b/drivers/s390/Makefile
> @@ -3,7 +3,7 @@
> # Makefile for the S/390 specific device drivers
> #
>
> -obj-y += cio/ block/ char/ crypto/ net/ scsi/ virtio/
> -
> -drivers-y += drivers/s390/built-in.a
> -
> +ifeq ($(ARCH),s390)
> + obj-y += cio/ block/ char/ crypto/ net/ scsi/ virtio/
> + drivers-y += drivers/s390/built-in.a
> +endif

And then somebody wants to build with e.g. "make drivers/s390/cio/" and it
still doesn't work. So _if_ this should be supported then it should work
with all directory levels and all configuration options. Otherwise this is
going to be a never ending story.


2018-10-03 15:47:37

by Leonardo Brás

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] Remove errors building drivers/DRIVERNAME

On Mon, Oct 1, 2018 at 4:56 AM Robert Richter <[email protected]> wrote:
>
> On 27.09.18 23:08:09, Leonardo Brás wrote:
> > This Patchset changes some driver's Makefile to allow them building
> > using the command 'make drivers/DRIVERNAME', if compatible.
> >
> > The changed drivers would return error if the above command was run
> > on them, after an x86 allyesconfig.
>
> I don't see what you are trying to achieve here. Why shouldn't the
> command fail if it is not the intended way to call it? There are a
> couple of use cases where drivers/ is used to share common code over
> different archs and it is not always the intention to build them in
> drivers/ directly.


Sorry, I was not very clear at my reasons why this change is important,
I will try to briefly explain the whole story.

Some weeks ago I was trying to solve a task that needed to change some
compiling options, build the whole kernel (allyesconfig) and look for errors.
The problem was: It would take a long time to build everything in my computer.
And many friends with slimmer laptops would take much longer.

So, I was looking for a service that could do that for me, in the cloud.
I found out Gitlab.com offers free 50k minutes of CI for open source projects,
and allows anyone get this CI time by only forking my project, adding their
changes and pushing to Gitlab.

But Gitlab don't allow 'jobs' to take longer than 3 hours, after that the 'job'
is killed.

The kernel could not be built in 3h, not with allyesconfig. So, I created a
'job' for each directory in Linux kernel, and tried to build them separately.
All went fine, except for drivers/, which took over 3 hours.

Most logical thing was to continue the division and create five 'jobs' that
could divide the building time of drivers. To do that, each job took care of a
range of starting letters, as you can see in this link:
https://gitlab.com/LeoBras/linux-next/blob/build-ci/.gitlab-ci.yml

But then I found out some drivers were failing to build. Even if they were not
enabled in my .config. After some work I found out some drivers selection is
done in drivers/Makefile, and incompatible drivers would break my build if
tried to call them directly on drivers/DRIVERNAME.

This patchset intents to let the .config selection happen also in
drivers/DRIVERNAME/Makefile, avoiding accidentally building drivers
that are not in .config.

This would allow the kernel to be build on Gitlab CI, and would benefit many
people who wants to help in the kernel development, but have not much
processing power in their machines.

I understand my changes may have mistakes, and I am trying to fix them all.
I thank you any suggestion to make the code better.

Also, I would be happy to know of any other solution to remote build my changes
and look for warnkings / errors.


Thank you for reading,
Leonardo Bras

2018-10-03 23:27:50

by Finn Thain

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] Remove errors building drivers/DRIVERNAME

On Wed, 3 Oct 2018, Leonardo Bras wrote:

>
> Sorry, I was not very clear at my reasons why this change is important,
> I will try to briefly explain the whole story.
>
> Some weeks ago I was trying to solve a task that needed to change some
> compiling options, build the whole kernel (allyesconfig) and look for
> errors. The problem was: It would take a long time to build everything
> in my computer. And many friends with slimmer laptops would take much
> longer.
>

It seems to me that you shouldn't need expensive optimization for
continuous integration. In theory that could make a big difference though
I admit to no experience of build farms.

Have you looked at ccache? It will save you from having to recompile any
files not changed.

You could also leverage all of your laptops together using distcc.

HTH.

--

2018-10-04 00:32:19

by Leonardo Brás

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

On Fri, Sep 28, 2018 at 4:15 AM James Bottomley
<[email protected]> wrote:
>
> On Thu, 2018-09-27 at 23:08 -0300, Leonardo Brás wrote:
> > Avoids building driver if 'make drivers/parisc/' is called and
> > CONFIG_PARISC is disabled.
>
> Is that really a problem? The drivers/Makefile has this:
>
> obj-$(CONFIG_PARISC) += parisc/
> And you just overrode that by forcing the build. It's not even clear
> we should refuse the build in that case; how would we know you don't
> have a legitimate reason for the override?
>

Sorry I did not explained my reasons earlier. I sent everybody involved an
e-mail explaining the full reason of this change.
(For reference it's here: https://lkml.org/lkml/2018/10/3/707)

> Signed-off-by: Leonardo Brás <[email protected]>
> > ---
> > drivers/parisc/Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
> > index 3cd5e6cb8478..80049d763aa0 100644
> > --- a/drivers/parisc/Makefile
> > +++ b/drivers/parisc/Makefile
> > @@ -24,5 +24,5 @@ obj-$(CONFIG_EISA) += eisa.o
> > eisa_enumerator.o eisa_eeprom.o
> > obj-$(CONFIG_SUPERIO) += superio.o
> > obj-$(CONFIG_CHASSIS_LCD_LED) += led.o
> > obj-$(CONFIG_PDC_STABLE) += pdc_stable.o
> > -obj-y += power.o
> > +obj-$(CONFIG_PARISC) += power.o
>
> If we conclude the use case is legitimate, that's not enough: the two
> inner symbols are PARISC only but CONFIG_EISA isn't.

You are right.
It worked for my needs because I am only building the drivers, and not
linking them. But i believe doing something like I did in zorro/Makefile
would fix this all. (For reference, https://lkml.org/lkml/2018/9/28/150 )

If you agree, I will send the next patchset with this change.

Thanks for your help!

Leonardo Bras

2018-10-04 01:02:40

by Leonardo Brás

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] drivers: s390: Avoids building drivers if ARCH is not s390.

On Mon, Oct 1, 2018 at 9:46 AM Heiko Carstens <[email protected]> wrote:
>
> On Thu, Sep 27, 2018 at 11:08:14PM -0300, Leonardo Brás wrote:
> > Avoids building s390 drivers if 'make drivers/s390/' is called but
> > ARCH is not s390.
> >
> > Signed-off-by: Leonardo Brás <[email protected]>
> > ---
> > drivers/s390/Makefile | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/s390/Makefile b/drivers/s390/Makefile
> > index a863b0462b43..0575f02dba45 100644
> > --- a/drivers/s390/Makefile
> > +++ b/drivers/s390/Makefile
> > @@ -3,7 +3,7 @@
> > # Makefile for the S/390 specific device drivers
> > #
> >
> > -obj-y += cio/ block/ char/ crypto/ net/ scsi/ virtio/
> > -
> > -drivers-y += drivers/s390/built-in.a
> > -
> > +ifeq ($(ARCH),s390)
> > + obj-y += cio/ block/ char/ crypto/ net/ scsi/ virtio/
> > + drivers-y += drivers/s390/built-in.a
> > +endif
>
> And then somebody wants to build with e.g. "make drivers/s390/cio/" and it
> still doesn't work. So _if_ this should be supported then it should work
> with all directory levels and all configuration options. Otherwise this is
> going to be a never ending story.
>


It makes sense.
I proposed this change to help me solving a problem described here
(https://lkml.org/lkml/2018/10/3/707), and for this it was enough if it didn't
build when "make drivers/s390/" was called.

Sorry I didn't send the e-mail with the reason earlier.

For solving my problem it was not necessary, but if you think it's interesting,
I could refactor all drivers/s390 Makefiles to make them all build only if
we are dealing with the s390 architecture.

What do you think?

Thanks for the reply,

Leonardo Bras

2018-10-04 01:39:17

by Leonardo Brás

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] Remove errors building drivers/DRIVERNAME

On Wed, Oct 3, 2018 at 8:27 PM Finn Thain <[email protected]> wrote:
>
> On Wed, 3 Oct 2018, Leonardo Bras wrote:
>
> >
> > Sorry, I was not very clear at my reasons why this change is important,
> > I will try to briefly explain the whole story.
> >
> > Some weeks ago I was trying to solve a task that needed to change some
> > compiling options, build the whole kernel (allyesconfig) and look for
> > errors. The problem was: It would take a long time to build everything
> > in my computer. And many friends with slimmer laptops would take much
> > longer.
> >
>
> It seems to me that you shouldn't need expensive optimization for
> continuous integration. In theory that could make a big difference though
> I admit to no experience of build farms.
>
> Have you looked at ccache? It will save you from having to recompile any
> files not changed.
>
> You could also leverage all of your laptops together using distcc.
>
> HTH.
>
> --

Both ccache and distcc seem very interesting,
I will take my time to study them better as they can solve some situations
I face. Thanks for sharing!

Still, I believe my patchset is relevant, as it would enable people to rebuild
the whole kernel (with build flags changes, for example) at the cost of only
git-pushing the changes to a fork of my Gitlab repo, and waiting 3 hours.

Thanks for helping!

2018-10-04 02:01:04

by Finn Thain

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] Remove errors building drivers/DRIVERNAME

On Wed, 3 Oct 2018, Leonardo Bras wrote:

> Both ccache and distcc seem very interesting, I will take my time to
> study them better as they can solve some situations I face. Thanks for
> sharing!
>

You might also want to check out 'gcc -O0', 'gcc -fopt-info' and 'gcc
--help=optimizers' etc to see if you can reduce the compute cost.

To reduce IO cost, my build tests always use 'make O=/some/path' where
/some/path is on a tmpfs mountpoint.

HTH.

--

2018-10-04 04:41:29

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

On Wed, 2018-10-03 at 21:31 -0300, Leonardo Bras wrote:
> On Fri, Sep 28, 2018 at 4:15 AM James Bottomley
> <[email protected]> wrote:
> >
> > On Thu, 2018-09-27 at 23:08 -0300, Leonardo Brás wrote:
> > > Avoids building driver if 'make drivers/parisc/' is called and
> > > CONFIG_PARISC is disabled.
> >
> > Is that really a problem? The drivers/Makefile has this:
> >
> > obj-$(CONFIG_PARISC)            += parisc/
> > And you just overrode that by forcing the build.  It's not even
> > clear we should refuse the build in that case; how would we know
> > you don't have a legitimate reason for the override?
> >
>
> Sorry I did not explained my reasons earlier. I sent everybody
> involved an e-mail explaining the full reason of this change.
> (For reference it's here: https://lkml.org/lkml/2018/10/3/707)

Well it's not really that persuasive. Most people simply let the build
run to completion, but if you have a problem with a job control 3h
timelimit, then create a job that kills itself at 2:59 and then
resubmits itself. That will produce a complete build in 3h chunks
without any need to call sub Makefiles.

All of our Makefiles are coded assuming the upper level can prevent
descent into the lower ones. You're proposing to change that
assumption, requiring a fairly large patch set, which doesn't really
seem to provide a huge benefit.

James


> > Signed-off-by: Leonardo Brás <[email protected]>
> > > ---
> > >  drivers/parisc/Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/parisc/Makefile b/drivers/parisc/Makefile
> > > index 3cd5e6cb8478..80049d763aa0 100644
> > > --- a/drivers/parisc/Makefile
> > > +++ b/drivers/parisc/Makefile
> > > @@ -24,5 +24,5 @@ obj-$(CONFIG_EISA)          += eisa.o
> > > eisa_enumerator.o eisa_eeprom.o
> > >  obj-$(CONFIG_SUPERIO)                += superio.o
> > >  obj-$(CONFIG_CHASSIS_LCD_LED)        += led.o
> > >  obj-$(CONFIG_PDC_STABLE)     += pdc_stable.o
> > > -obj-y                                += power.o
> > > +obj-$(CONFIG_PARISC)         += power.o
> >
> > If we conclude the use case is legitimate, that's not enough: the
> > two
> > inner symbols are PARISC only but CONFIG_EISA isn't.
>
> You are right.
> It worked for my needs because I am only building the drivers, and
> not linking them. But i believe doing something like I did in
> zorro/Makefile would fix this all. (For reference,
> https://lkml.org/lkml/2018/9/28/150 )
>
> If you agree, I will send the next patchset with this change.
>
> Thanks for your help!
>
> Leonardo Bras
>


2018-10-05 02:17:00

by Leonardo Brás

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

> Well it's not really that persuasive. Most people simply let the build
> run to completion, but if you have a problem with a job control 3h
> timelimit, then create a job that kills itself at 2:59 and then
> resubmits itself. That will produce a complete build in 3h chunks
> without any need to call sub Makefiles.
>

Humm, I probably should have explained better how GitlabCI works.
It works creating a docker container that have a limited lifespan of 3h max.
After that the time is over, this container ceases to exist, leaving no build
objects, only the console log. So there is no way of 'resuming' the building
from where it stopped. I used the 'job' term because it's how they call it,
and I understand it's easily confused with bash jobs.

> All of our Makefiles are coded assuming the upper level can prevent
> descent into the lower ones. You're proposing to change that
> assumption, requiring a fairly large patch set, which doesn't really
> seem to provide a huge benefit.
>
> James

I understand your viewpoint.
But what I propose is not to change that assumption, but instead give some
Makefiles the aditional ability to be called directly and still not build stuff
if they were not enabled in .config.

But, why these chosen Makefiles, and not all of them?
Granularity.
What I am trying to achieve with this patchset is the ability of building
smaller sets of drivers without accidentally building what is not enabled
on .config.
And, in my viewpoint, building a single drivers/DRIVERNAME is small enough to
be fast in most situations.

This change is not supposed to bother the usual way of building the kernel, and
it is not even supposed to add overhead to kernel compilation. And it would,
at least, solve my problem with the 3h limit, and enable the tool
I am building on GiltabCI to help other developers.

Thanks for reading,

Leonardo Bras

2018-10-05 04:10:30

by Finn Thain

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

On Thu, 4 Oct 2018, Leonardo Bras wrote:

> ...
> But, why these chosen Makefiles, and not all of them?

I think that inconsistency is untenable. If nothing else, it means your
changes will get broken by other people who also apply constraints
inconsistently.

I think you need to consider what happens when you apply this regime
tree-wide. Either that or explain why it would make sense to build e.g.
drivers/s390 but not drivers/s390/net.

--

2018-10-06 04:34:37

by Michael Schmitz

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled



Am 05.10.2018 um 15:16 schrieb Leonardo Bras:
>> Well it's not really that persuasive. Most people simply let the build
>> run to completion, but if you have a problem with a job control 3h
>> timelimit, then create a job that kills itself at 2:59 and then
>> resubmits itself. That will produce a complete build in 3h chunks
>> without any need to call sub Makefiles.
>>
>
> Humm, I probably should have explained better how GitlabCI works.
> It works creating a docker container that have a limited lifespan of 3h max.
> After that the time is over, this container ceases to exist, leaving no build
> objects, only the console log. So there is no way of 'resuming' the building
> from where it stopped. I used the 'job' term because it's how they call it,
> and I understand it's easily confused with bash jobs.
>
>> All of our Makefiles are coded assuming the upper level can prevent
>> descent into the lower ones. You're proposing to change that
>> assumption, requiring a fairly large patch set, which doesn't really
>> seem to provide a huge benefit.
>>
>> James
>
> I understand your viewpoint.
> But what I propose is not to change that assumption, but instead give some
> Makefiles the aditional ability to be called directly and still not build stuff
> if they were not enabled in .config.
>
> But, why these chosen Makefiles, and not all of them?
> Granularity.
> What I am trying to achieve with this patchset is the ability of building
> smaller sets of drivers without accidentally building what is not enabled
> on .config.
> And, in my viewpoint, building a single drivers/DRIVERNAME is small enough to
> be fast in most situations.

That already works, doesn't it? So all that you'd need is an offline
tool to precompute what drivers to actually build with a given config.

'make -n' with some suitable output mangling might do the job.

There may well be other ways to achieve your stated goal, without any
need to make changes to the kernel build process (which is the result of
many years of evolution and tuning, BTW).

> This change is not supposed to bother the usual way of building the kernel, and

Enough people have voiced their concern to warrant that you should back
up that claim, IMO. Have you verified that your patchset does not change
current behaviour when building the entire set of default configurations
for each supported architecture? Does it reduce or increase overall
complexity of the build process?

> it is not even supposed to add overhead to kernel compilation. And it would,
> at least, solve my problem with the 3h limit, and enable the tool
> I am building on GiltabCI to help other developers.

(Apropos of nothing: Am I the only one who thinks gitlab might take a
rather dim view of your creativity in dealing with their limit?)

> Thanks for reading,
>
> Leonardo Bras

Thanks for listening!

Cheers,

Michael

2018-10-10 01:05:27

by Leonardo Brás

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] Remove errors building drivers/DRIVERNAME

Thanks Finn, I will take a good look at that and try to use it in my build.

Thank you,
Leonardo Bras

On Wed, Oct 3, 2018 at 11:00 PM Finn Thain <[email protected]> wrote:
>
> On Wed, 3 Oct 2018, Leonardo Bras wrote:
>
> > Both ccache and distcc seem very interesting, I will take my time to
> > study them better as they can solve some situations I face. Thanks for
> > sharing!
> >
>
> You might also want to check out 'gcc -O0', 'gcc -fopt-info' and 'gcc
> --help=optimizers' etc to see if you can reduce the compute cost.
>
> To reduce IO cost, my build tests always use 'make O=/some/path' where
> /some/path is on a tmpfs mountpoint.
>
> HTH.
>
> --

2018-10-10 01:16:21

by Leonardo Brás

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] drivers: parisc: Avoids building driver if CONFIG_PARISC is disabled

Hello Michael,

> That already works, doesn't it? So all that you'd need is an offline
> tool to precompute what drivers to actually build with a given config.
>
> 'make -n' with some suitable output mangling might do the job.
>
> There may well be other ways to achieve your stated goal, without any
> need to make changes to the kernel build process (which is the result of
> many years of evolution and tuning, BTW).

Thanks for the info, I will try to use it.

> > This change is not supposed to bother the usual way of building the kernel, and
>
> Enough people have voiced their concern to warrant that you should back
> up that claim, IMO. Have you verified that your patchset does not change
> current behaviour when building the entire set of default configurations
> for each supported architecture? Does it reduce or increase overall
> complexity of the build process?
>
I have tried in some ARCHs and it worked fine. Out of curiosity, I
will try on all
of them.

> > it is not even supposed to add overhead to kernel compilation. And it would,
> > at least, solve my problem with the 3h limit, and enable the tool
> > I am building on GiltabCI to help other developers.
>
> (Apropos of nothing: Am I the only one who thinks gitlab might take a
> rather dim view of your creativity in dealing with their limit?)
>

They make available 50k minutes a month for OSS projects. I don't believe they
care how it's spent if its used to build/deploy the project. They even
allow using
several 'jobs' in parallel in order to speed up the process.

Thanks for your help,

Leonardo Bras