arch/s390/kvm/kvm-s390.c calls on several places __cpacf_query() directly,
which makes it impossible to meet the "i" constraint for the asm operands
(opcode in this case).
As we are now force-enabling CONFIG_OPTIMIZE_INLINING on all
architectures, this causes a build failure on s390:
In file included from arch/s390/kvm/kvm-s390.c:44:
./arch/s390/include/asm/cpacf.h: In function '__cpacf_query':
./arch/s390/include/asm/cpacf.h:179:2: warning: asm operand 3 probably doesn't match constraints
179 | asm volatile(
| ^~~
./arch/s390/include/asm/cpacf.h:179:2: error: impossible constraint in 'asm'
Mark __cpacf_query() as __always_inline in order to fix that, analogically
how we fixes __cpacf_check_opcode(), cpacf_query_func() and scpacf_query()
already.
Reported-and-tested-by: Michal Kubecek <[email protected]>
Fixes: d83623c5eab2 ("s390: mark __cpacf_check_opcode() and cpacf_query_func() as __always_inline")
Fixes: e60fb8bf68d4 ("s390/cpacf: mark scpacf_query() as __always_inline")
Fixes: ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING")
Signed-off-by: Jiri Kosina <[email protected]>
---
I am wondering how is it possible that none of the build-testing
infrastructure we have running against linux-next caught this? Not enough
non-x86 coverage?
arch/s390/include/asm/cpacf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
index a092f63aac6a..c0f3bfeddcbe 100644
--- a/arch/s390/include/asm/cpacf.h
+++ b/arch/s390/include/asm/cpacf.h
@@ -171,7 +171,7 @@ typedef struct { unsigned char bytes[16]; } cpacf_mask_t;
*
* Returns 1 if @func is available for @opcode, 0 otherwise
*/
-static inline void __cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
+static __always_inline void __cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
{
register unsigned long r0 asm("0") = 0; /* query function */
register unsigned long r1 asm("1") = (unsigned long) mask;
--
Jiri Kosina
SUSE Labs
On Tue, 1 Oct 2019, Jiri Kosina wrote:
> arch/s390/kvm/kvm-s390.c calls on several places __cpacf_query() directly,
> which makes it impossible to meet the "i" constraint for the asm operands
> (opcode in this case).
>
> As we are now force-enabling CONFIG_OPTIMIZE_INLINING on all
> architectures, this causes a build failure on s390:
>
> In file included from arch/s390/kvm/kvm-s390.c:44:
> ./arch/s390/include/asm/cpacf.h: In function '__cpacf_query':
> ./arch/s390/include/asm/cpacf.h:179:2: warning: asm operand 3 probably doesn't match constraints
> 179 | asm volatile(
> | ^~~
> ./arch/s390/include/asm/cpacf.h:179:2: error: impossible constraint in 'asm'
>
> Mark __cpacf_query() as __always_inline in order to fix that, analogically
> how we fixes __cpacf_check_opcode(), cpacf_query_func() and scpacf_query()
> already.
>
> Reported-and-tested-by: Michal Kubecek <[email protected]>
> Fixes: d83623c5eab2 ("s390: mark __cpacf_check_opcode() and cpacf_query_func() as __always_inline")
> Fixes: e60fb8bf68d4 ("s390/cpacf: mark scpacf_query() as __always_inline")
> Fixes: ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
> Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING")
> Signed-off-by: Jiri Kosina <[email protected]>
Gah, due to bug in my script the sigoff doesn't match the From:, so
whoever is potentially applying it, please ammend it with
From: Jiri Kosina <[email protected]>
Sorry for the noise.
--
Jiri Kosina
SUSE Labs
On 01.10.19 22:08, Jiri Kosina wrote:
> arch/s390/kvm/kvm-s390.c calls on several places __cpacf_query() directly,
> which makes it impossible to meet the "i" constraint for the asm operands
> (opcode in this case).
>
> As we are now force-enabling CONFIG_OPTIMIZE_INLINING on all
> architectures, this causes a build failure on s390:
>
> In file included from arch/s390/kvm/kvm-s390.c:44:
> ./arch/s390/include/asm/cpacf.h: In function '__cpacf_query':
> ./arch/s390/include/asm/cpacf.h:179:2: warning: asm operand 3 probably doesn't match constraints
> 179 | asm volatile(
> | ^~~
> ./arch/s390/include/asm/cpacf.h:179:2: error: impossible constraint in 'asm'
>
> Mark __cpacf_query() as __always_inline in order to fix that, analogically
> how we fixes __cpacf_check_opcode(), cpacf_query_func() and scpacf_query()
> already.
>
> Reported-and-tested-by: Michal Kubecek <[email protected]>
> Fixes: d83623c5eab2 ("s390: mark __cpacf_check_opcode() and cpacf_query_func() as __always_inline")
> Fixes: e60fb8bf68d4 ("s390/cpacf: mark scpacf_query() as __always_inline")
> Fixes: ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
> Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING")
> Signed-off-by: Jiri Kosina <[email protected]>
Thanks applied.
>
> I am wondering how is it possible that none of the build-testing
> infrastructure we have running against linux-next caught this? Not enough
> non-x86 coverage?
We do build-test linux-next daily. Maybe our compiler just made a different
inlining decision.
On Tue, Oct 01, 2019 at 10:08:01PM +0200, Jiri Kosina wrote:
> arch/s390/kvm/kvm-s390.c calls on several places __cpacf_query() directly,
> which makes it impossible to meet the "i" constraint for the asm operands
> (opcode in this case).
>
> As we are now force-enabling CONFIG_OPTIMIZE_INLINING on all
> architectures, this causes a build failure on s390:
>
> In file included from arch/s390/kvm/kvm-s390.c:44:
> ./arch/s390/include/asm/cpacf.h: In function '__cpacf_query':
> ./arch/s390/include/asm/cpacf.h:179:2: warning: asm operand 3 probably doesn't match constraints
> 179 | asm volatile(
> | ^~~
> ./arch/s390/include/asm/cpacf.h:179:2: error: impossible constraint in 'asm'
>
> Mark __cpacf_query() as __always_inline in order to fix that, analogically
> how we fixes __cpacf_check_opcode(), cpacf_query_func() and scpacf_query()
> already.
>
> Reported-and-tested-by: Michal Kubecek <[email protected]>
> Fixes: d83623c5eab2 ("s390: mark __cpacf_check_opcode() and cpacf_query_func() as __always_inline")
> Fixes: e60fb8bf68d4 ("s390/cpacf: mark scpacf_query() as __always_inline")
> Fixes: ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
> Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING")
> Signed-off-by: Jiri Kosina <[email protected]>
> ---
>
> I am wondering how is it possible that none of the build-testing
> infrastructure we have running against linux-next caught this? Not enough
> non-x86 coverage?
Well, there is plenty of s390 coverage with respect to daily builds
(also here). It doesn't fail for me with gcc 9.1; so you may either
have a different gcc version or different config options(?) so the
compiler decided to not inline the function. I'll apply this in any
case, since your patch is obviously fine.
Thanks!
> arch/s390/include/asm/cpacf.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
> index a092f63aac6a..c0f3bfeddcbe 100644
> --- a/arch/s390/include/asm/cpacf.h
> +++ b/arch/s390/include/asm/cpacf.h
> @@ -171,7 +171,7 @@ typedef struct { unsigned char bytes[16]; } cpacf_mask_t;
> *
> * Returns 1 if @func is available for @opcode, 0 otherwise
> */
> -static inline void __cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
> +static __always_inline void __cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
> {
> register unsigned long r0 asm("0") = 0; /* query function */
> register unsigned long r1 asm("1") = (unsigned long) mask;
>
> --
> Jiri Kosina
> SUSE Labs
>
On Wed, Oct 02, 2019 at 08:46:05AM +0200, Heiko Carstens wrote:
> On Tue, Oct 01, 2019 at 10:08:01PM +0200, Jiri Kosina wrote:
> > I am wondering how is it possible that none of the build-testing
> > infrastructure we have running against linux-next caught this? Not enough
> > non-x86 coverage?
>
> Well, there is plenty of s390 coverage with respect to daily builds
> (also here). It doesn't fail for me with gcc 9.1; so you may either
> have a different gcc version or different config options(?) so the
> compiler decided to not inline the function. I'll apply this in any
> case, since your patch is obviously fine.
>
> Thanks!
Ok, Christian applied this already a couple of minutes earlier ;)
On Wed, Oct 02, 2019 at 08:46:05AM +0200, Heiko Carstens wrote:
> On Tue, Oct 01, 2019 at 10:08:01PM +0200, Jiri Kosina wrote:
> >
> > In file included from arch/s390/kvm/kvm-s390.c:44:
> > ./arch/s390/include/asm/cpacf.h: In function '__cpacf_query':
> > ./arch/s390/include/asm/cpacf.h:179:2: warning: asm operand 3 probably doesn't match constraints
> > 179 | asm volatile(
> > | ^~~
> > ./arch/s390/include/asm/cpacf.h:179:2: error: impossible constraint in 'asm'
> >
> > ...
> >
> > I am wondering how is it possible that none of the build-testing
> > infrastructure we have running against linux-next caught this? Not enough
> > non-x86 coverage?
>
> Well, there is plenty of s390 coverage with respect to daily builds
> (also here). It doesn't fail for me with gcc 9.1; so you may either
> have a different gcc version or different config options(?) so the
> compiler decided to not inline the function.
I think I found the reason: we only hit the build failure with one
special config used for zfcpdump which has
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
When I switched to CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y (which we have
in other s390x configs and which most people probably prefer), the build
does not fail even without the patch.
Michal Kubecek
On Wed, Oct 02, 2019 at 09:03:33AM +0200, Michal Kubecek wrote:
> On Wed, Oct 02, 2019 at 08:46:05AM +0200, Heiko Carstens wrote:
> > On Tue, Oct 01, 2019 at 10:08:01PM +0200, Jiri Kosina wrote:
> > >
> > > In file included from arch/s390/kvm/kvm-s390.c:44:
> > > ./arch/s390/include/asm/cpacf.h: In function '__cpacf_query':
> > > ./arch/s390/include/asm/cpacf.h:179:2: warning: asm operand 3 probably doesn't match constraints
> > > 179 | asm volatile(
> > > | ^~~
> > > ./arch/s390/include/asm/cpacf.h:179:2: error: impossible constraint in 'asm'
> > >
> > > ...
> > >
> > > I am wondering how is it possible that none of the build-testing
> > > infrastructure we have running against linux-next caught this? Not enough
> > > non-x86 coverage?
> >
> > Well, there is plenty of s390 coverage with respect to daily builds
> > (also here). It doesn't fail for me with gcc 9.1; so you may either
> > have a different gcc version or different config options(?) so the
> > compiler decided to not inline the function.
>
> I think I found the reason: we only hit the build failure with one
> special config used for zfcpdump which has
>
> CONFIG_CC_OPTIMIZE_FOR_SIZE=y
>
> When I switched to CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y (which we have
> in other s390x configs and which most people probably prefer), the build
> does not fail even without the patch.
Yes, with CONFIG_CC_OPTIMIZE_FOR_SIZE=y I can see plenty of _additional_
compile failures on s390 with "defconfig". Will fix them all...
Thanks for reporting!