[PATCH v1 1/1] drm: fix alpha build after drm_util.h change

[PATCH v1 1/1] drm: fix alpha build after drm_util.h change

[Sam Ravnborg]

0-DAY reported the following bug:

tree:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
head:   21376e2c3c5bad5e87ba700c055c8a8235c2bfd5
commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move drm_can_sleep() to drm_util.h
config: alpha-allmodconfig (attached as .config)
...
   In file included from include/linux/irqflags.h:16:0,
                    from include/drm/drm_util.h:35,
                    from drivers/gpu/drm/qxl/qxl_cmd.c:28:
>> arch/alpha/include/asm/irqflags.h:58:15: error: unknown type name 'bool'
    static inline bool arch_irqs_disabled_flags(unsigned long flags)
                  ^~~~
...

So we have a situation where we do not pull in <linux/types.h>
when building for alpha.

An quick grep shows that roughly half of the declarations of
arch_irqs_disabled_flags() uses int as return type, the other half bool.

Two invasive fixes where considered:
- Change all declarations of arch_irqs_disabled_flags() to use bool
- Add include of <linux/types.h> to all files that uses bool for
  arch_irqs_disabled_flags

To invases with a too high pain/benefit ratio, so dropped.

Some less invasive fixes was also considered:
- Add include of <linux/types.h> to irqflags.h
- Add include of <linux/types.h> to qxl_cmd.c
- Add include of <linux/types.h> to drm_util.h

The first was dropped as irqflags.h did not use bool, so no need for the
types.h header file.

The latter was considered the best option as there could
be other similar cases and we would like the header files below
include/drm/ to be selfcontained.

It turns out that using the standard alphabetical order did not work
well as we then included irqflags.h before types.h.
It was considered just to pull in interrupt.h but that would
pull in a lot of unused stuff.
So in the end types.h was included with a comment that it must be first.

Build tested with alpha allmodconfig.

Fixes: 733748ac37b45 ("drm: move drm_can_sleep() to drm_util.h")
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 include/drm/drm_util.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
index 0500da65b1d1..0f550bc91db1 100644
--- a/include/drm/drm_util.h
+++ b/include/drm/drm_util.h
@@ -32,6 +32,8 @@
  * Macros and inline functions that does not naturally belong in other places
  */
 
+#include <linux/types.h>  /* Due to header dependencies this must be first */
+
 #include <linux/irqflags.h>
 #include <linux/preempt.h>
 #include <linux/kgdb.h>
-- 
2.12.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/1] drm: fix alpha build after drm_util.h change

[Sam Ravnborg]

Hi all.

Please do not apply.

O-DAY found a similar bug when building ia64, but this time there is
something else from irqflags.h that causes the error.

So...
> It was considered just to pull in interrupt.h but that would
> pull in a lot of unused stuff.

This seems to be the rigth solution anyway, as interrupt.h
have all the necessary dependencies.
I will try this tonight and post an updated patch.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/1] drm: fix alpha build after drm_util.h change

[Sam Ravnborg]

0-DAY reported the following bug:

tree:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
head:   21376e2c3c5bad5e87ba700c055c8a8235c2bfd5
commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move drm_can_sleep() to drm_util.h
config: alpha-allmodconfig (attached as .config)
...
   In file included from include/linux/irqflags.h:16:0,
                    from include/drm/drm_util.h:35,
                    from drivers/gpu/drm/qxl/qxl_cmd.c:28:
>> arch/alpha/include/asm/irqflags.h:58:15: error: unknown type name 'bool'
    static inline bool arch_irqs_disabled_flags(unsigned long flags)
                  ^~~~

And later following bug:
tree:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
head:   21376e2c3c5bad5e87ba700c055c8a8235c2bfd5
commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move drm_can_sleep() to drm_util.h
config: ia64-allyesconfig (attached as .config)
...
   In file included from arch/ia64/include/asm/irqflags.h:14,
                    from include/linux/irqflags.h:16,
                    from include/drm/drm_util.h:35,
                    from drivers/gpu/drm/qxl/qxl_cmd.c:28:
   arch/ia64/include/asm/pal.h: In function 'ia64_pal_tr_read':
   arch/ia64/include/asm/pal.h:1703:64: error: implicit declaration of function 'ia64_tpa'; did you mean 'ia64_pal'?  [-Werror=implicit-function-declaration]
     PAL_CALL_PHYS_STK(iprv, PAL_VM_TR_READ, reg_num, tr_type,(u64)ia64_tpa(tr_buffer));
                                                                   ^~~~~~~~
...

So we have a situation where we do not pull in <linux/types.h>
when building for alpha and for ia64 we need even more definitions
are required.

Two invasive fixes where considered:
- Change all declarations of arch_irqs_disabled_flags() to use bool
- Add include of <linux/types.h> to all files that uses bool for
  arch_irqs_disabled_flags

To invasive with a too high pain/benefit ratio, so dropped.
They would not cover ia64 either.

Some less invasive fixes was also considered:
- Add include of <linux/types.h> to drm_util.h
- Add include of <linux/interrupt.h> to drm_util.h

The first was dropped as this did not cover the ia64 case.

The latter was considered the best option as there could
be other similar cases and we would like the header files below
include/drm/ to be selfcontained.
So we end up pulling in a lot of stuff not needed, but this is
the price we pay in drm/ because the kernel headers are not all
selfcontained.

While at it, ordred the includefiles in drm_util in alphabetical order.

Build tested with alpha,ia64,arm,x86 with allmodconfig and allyesconfig.

v2:
- fix ia64 build, changed to include interrupt.h
- sort include files alphabetically

Fixes: 733748ac37b45 ("drm: move drm_can_sleep() to drm_util.h")
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 include/drm/drm_util.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
index 0500da65b1d1..8163d35f8327 100644
--- a/include/drm/drm_util.h
+++ b/include/drm/drm_util.h
@@ -32,9 +32,9 @@
  * Macros and inline functions that does not naturally belong in other places
  */
 
-#include <linux/irqflags.h>
-#include <linux/preempt.h>
+#include <linux/interrupt.h>
 #include <linux/kgdb.h>
+#include <linux/preempt.h>
 #include <linux/smp.h>
 
 /*
-- 
2.12.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/1] drm: fix alpha build after drm_util.h change

[Daniel Vetter]

On Tue, Jan 15, 2019 at 10:48:45PM +0100, Sam Ravnborg wrote:
> 0-DAY reported the following bug:
> 
> tree:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> head:   21376e2c3c5bad5e87ba700c055c8a8235c2bfd5
> commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move drm_can_sleep() to drm_util.h
> config: alpha-allmodconfig (attached as .config)
> ...
>    In file included from include/linux/irqflags.h:16:0,
>                     from include/drm/drm_util.h:35,
>                     from drivers/gpu/drm/qxl/qxl_cmd.c:28:
> >> arch/alpha/include/asm/irqflags.h:58:15: error: unknown type name 'bool'
>     static inline bool arch_irqs_disabled_flags(unsigned long flags)
>                   ^~~~
> 
> And later following bug:
> tree:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> head:   21376e2c3c5bad5e87ba700c055c8a8235c2bfd5
> commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move drm_can_sleep() to drm_util.h
> config: ia64-allyesconfig (attached as .config)
> ...
>    In file included from arch/ia64/include/asm/irqflags.h:14,
>                     from include/linux/irqflags.h:16,
>                     from include/drm/drm_util.h:35,
>                     from drivers/gpu/drm/qxl/qxl_cmd.c:28:
>    arch/ia64/include/asm/pal.h: In function 'ia64_pal_tr_read':
>    arch/ia64/include/asm/pal.h:1703:64: error: implicit declaration of function 'ia64_tpa'; did you mean 'ia64_pal'?  [-Werror=implicit-function-declaration]
>      PAL_CALL_PHYS_STK(iprv, PAL_VM_TR_READ, reg_num, tr_type,(u64)ia64_tpa(tr_buffer));
>                                                                    ^~~~~~~~
> ...
> 
> So we have a situation where we do not pull in <linux/types.h>
> when building for alpha and for ia64 we need even more definitions
> are required.
> 
> Two invasive fixes where considered:
> - Change all declarations of arch_irqs_disabled_flags() to use bool
> - Add include of <linux/types.h> to all files that uses bool for
>   arch_irqs_disabled_flags
> 
> To invasive with a too high pain/benefit ratio, so dropped.
> They would not cover ia64 either.
> 
> Some less invasive fixes was also considered:
> - Add include of <linux/types.h> to drm_util.h
> - Add include of <linux/interrupt.h> to drm_util.h
> 
> The first was dropped as this did not cover the ia64 case.
> 
> The latter was considered the best option as there could
> be other similar cases and we would like the header files below
> include/drm/ to be selfcontained.
> So we end up pulling in a lot of stuff not needed, but this is
> the price we pay in drm/ because the kernel headers are not all
> selfcontained.
> 
> While at it, ordred the includefiles in drm_util in alphabetical order.
> 
> Build tested with alpha,ia64,arm,x86 with allmodconfig and allyesconfig.
> 
> v2:
> - fix ia64 build, changed to include interrupt.h
> - sort include files alphabetically
> 
> Fixes: 733748ac37b45 ("drm: move drm_can_sleep() to drm_util.h")
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>

Applied, thanks for your patch.
-Daniel

> ---
>  include/drm/drm_util.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
> index 0500da65b1d1..8163d35f8327 100644
> --- a/include/drm/drm_util.h
> +++ b/include/drm/drm_util.h
> @@ -32,9 +32,9 @@
>   * Macros and inline functions that does not naturally belong in other places
>   */
>  
> -#include <linux/irqflags.h>
> -#include <linux/preempt.h>
> +#include <linux/interrupt.h>
>  #include <linux/kgdb.h>
> +#include <linux/preempt.h>
>  #include <linux/smp.h>
>  
>  /*
> -- 
> 2.12.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel