linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm: prevent inlining in arxescsi.c causing build failures
@ 2012-01-22 19:16 Paul Gortmaker
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Gortmaker @ 2012-01-22 19:16 UTC (permalink / raw)
  To: linux, JBottomley
  Cc: linux-arm-kernel, linux-scsi, linux-kernel, Paul Gortmaker,
	Arnd Bergmann, Uwe Kleine-König

The following failure is seen in routine coverage builds:

  CC [M]  drivers/scsi/arm/arxescsi.o
/tmp/cccEyiO7.s: Assembler messages:
/tmp/cccEyiO7.s:334: Error: symbol `.loop_1' is already defined
/tmp/cccEyiO7.s:337: Error: symbol `.loop_2' is already defined
/tmp/cccEyiO7.s:343: Error: symbol `.loop_3' is already defined
/tmp/cccEyiO7.s:365: Error: symbol `.loop_1' is already defined
/tmp/cccEyiO7.s:368: Error: symbol `.loop_2' is already defined
/tmp/cccEyiO7.s:374: Error: symbol `.loop_3' is already defined
make[4]: *** [drivers/scsi/arm/arxescsi.o] Error 1

It is caused by multiple inline of arxescsi_pseudo_dma_write()
which is responsible for the above labels.  Marking the fcn
as non-inline fixes the issue.

Arnd Bergmann considered un-inlinling vs fixing the loop[1] to
use local labels and came to the following conclusion:

  The problem is mostly in the code: The symbols are defined in an
  inline assembly that is not meant to be instantiated multiple times.
  gcc may decide to unroll the loop in which this is done, which causes
  the error above. Unrolling the loop is a rather silly thing to do
  here, because the code is rather large and does not at all benefit
  from unrolling. You can use the patch below to fix the code to still
  work if the loop is unrolled, but I also think that marking the
  function as uninline is a good idea, in particular because I don't
  trust the old assembly to still do the right thing otherwise:
  It manually saves and restores the registers on the stack, where
  you nowadays would specify specific clobbers for the registers
  it uses, and let the compiler take care of it. It also doesn't
  contain a memory clobber for the buffer, so keeping the function
  out-of-line is probably the safe choice.

[1] https://lkml.org/lkml/2011/12/1/189

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

---
v2: add desc. from Arnd indicating why un-inlining is the best choice

diff --git a/drivers/scsi/arm/arxescsi.c b/drivers/scsi/arm/arxescsi.c
index a750aa7..2608a9e 100644
--- a/drivers/scsi/arm/arxescsi.c
+++ b/drivers/scsi/arm/arxescsi.c
@@ -72,7 +72,8 @@ arxescsi_dma_setup(struct Scsi_Host *host, struct scsi_pointer *SCp,
 	return fasdma_pseudo;
 }
 
-static void arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
+static noinline void
+arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
 {
        __asm__ __volatile__(
        "               stmdb   sp!, {r0-r12}\n"
-- 
1.7.7.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm: prevent inlining in arxescsi.c causing build failures
  2011-12-01  8:06 ` Uwe Kleine-König
  2011-12-01 10:46   ` Sergei Shtylyov
@ 2011-12-01 14:06   ` Arnd Bergmann
  1 sibling, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2011-12-01 14:06 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Uwe Kleine-König, Paul Gortmaker, linux-kernel, JBottomley,
	linux-scsi

On Thursday 01 December 2011, Uwe Kleine-König wrote:
> On Wed, Nov 30, 2011 at 11:26:51PM -0500, Paul Gortmaker wrote:
> > The following failure is seen in routine coverage builds:
> > 
> >   CC [M]  drivers/scsi/arm/arxescsi.o
> > /tmp/cccEyiO7.s: Assembler messages:
> > /tmp/cccEyiO7.s:334: Error: symbol `.loop_1' is already defined
> > /tmp/cccEyiO7.s:337: Error: symbol `.loop_2' is already defined
> > /tmp/cccEyiO7.s:343: Error: symbol `.loop_3' is already defined
> > /tmp/cccEyiO7.s:365: Error: symbol `.loop_1' is already defined
> > /tmp/cccEyiO7.s:368: Error: symbol `.loop_2' is already defined
> > /tmp/cccEyiO7.s:374: Error: symbol `.loop_3' is already defined
> > make[4]: *** [drivers/scsi/arm/arxescsi.o] Error 1
> > 
> > It is caused by multiple inline of arxescsi_pseudo_dma_write()
> > which is responsible for the above labels.  Marking the fcn
> > as non-inline fixes the issue.
> > 
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Is that a compiler or a code issue? If it's the compiler please fix
> that. If it's the code, then please document why you added the noinline.

The problem is mostly in the code: The symbols are defined in an
inline assembly that is not meant to be instantiated multiple times.
gcc may decide to unroll the loop in which this is done, which causes
the error above. Unrolling the loop is a rather silly thing to do
here, because the code is rather large and does not at all benefit
from unrolling. You can use the patch below to fix the code to still
work if the loop is unrolled, but I also think that marking the
function as uninline is a good idea, in particular because I don't
trust the old assembly to still do the right thing otherwise:
It manually saves and restores the registers on the stack, where
you nowadays would specify specific clobbers for the registers
it uses, and let the compiler take care of it. It also doesn't
contain a memory clobber for the buffer, so keeping the function
out-of-line is probably the safe choice.

> Having said that, "my" compiler compiles drivers/scsi/arm/arxescsi.o
> just fine (using rpc_defconfig on v3.2-rc2).

You should be able to reproduce it by adding -funroll-all-loops to
the CFLAGS. That made the difference for me on gcc-4.6 from Ubuntu.

	Arnd

8<-----
PATCH: scsi/arxcescsi: use local assembler symbols

> > /tmp/cccEyiO7.s: Assembler messages:
> > /tmp/cccEyiO7.s:334: Error: symbol `.loop_1' is already defined
> > /tmp/cccEyiO7.s:337: Error: symbol `.loop_2' is already defined
> > /tmp/cccEyiO7.s:343: Error: symbol `.loop_3' is already defined
> > /tmp/cccEyiO7.s:365: Error: symbol `.loop_1' is already defined
> > /tmp/cccEyiO7.s:368: Error: symbol `.loop_2' is already defined
> > /tmp/cccEyiO7.s:374: Error: symbol `.loop_3' is already defined
> > make[4]: *** [drivers/scsi/arm/arxescsi.o] Error 1

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/scsi/arm/arxescsi.c b/drivers/scsi/arm/arxescsi.c
index a750aa7..689838a 100644
--- a/drivers/scsi/arm/arxescsi.c
+++ b/drivers/scsi/arm/arxescsi.c
@@ -80,21 +80,21 @@ static void arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
        "               mov     r1, %1\n"
        "               add     r2, r1, #512\n"
        "               mov     r4, #256\n"
-       ".loop_1:       ldmia   r3!, {r6, r8, r10, r12}\n"
+       "1:             ldmia   r3!, {r6, r8, r10, r12}\n"
        "               mov     r5, r6, lsl #16\n"
        "               mov     r7, r8, lsl #16\n"
-       ".loop_2:       ldrb    r0, [r1, #1536]\n"
+       "2:             ldrb    r0, [r1, #1536]\n"
        "               tst     r0, #1\n"
-       "               beq     .loop_2\n"
+       "               beq     2b\n"
        "               stmia   r2, {r5-r8}\n\t"
        "               mov     r9, r10, lsl #16\n"
        "               mov     r11, r12, lsl #16\n"
-       ".loop_3:       ldrb    r0, [r1, #1536]\n"
+       "3:             ldrb    r0, [r1, #1536]\n"
        "               tst     r0, #1\n"
-       "               beq     .loop_3\n"
+       "               beq     3b\n"
        "               stmia   r2, {r9-r12}\n"
        "               subs    r4, r4, #16\n"
-       "               bne     .loop_1\n"
+       "               bne     1b\n"
        "               ldmia   sp!, {r0-r12}\n"
        :
        : "r" (addr), "r" (base));

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm: prevent inlining in arxescsi.c causing build failures
  2011-12-01  8:06 ` Uwe Kleine-König
@ 2011-12-01 10:46   ` Sergei Shtylyov
  2011-12-01 14:06   ` Arnd Bergmann
  1 sibling, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2011-12-01 10:46 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Paul Gortmaker, linux-kernel, linux-arm-kernel, JBottomley, linux-scsi

Hello.

On 01.12.2011 12:06, Uwe Kleine-König wrote:

>> The following failure is seen in routine coverage builds:

>>    CC [M]  drivers/scsi/arm/arxescsi.o
>> /tmp/cccEyiO7.s: Assembler messages:
>> /tmp/cccEyiO7.s:334: Error: symbol `.loop_1' is already defined
>> /tmp/cccEyiO7.s:337: Error: symbol `.loop_2' is already defined
>> /tmp/cccEyiO7.s:343: Error: symbol `.loop_3' is already defined
>> /tmp/cccEyiO7.s:365: Error: symbol `.loop_1' is already defined
>> /tmp/cccEyiO7.s:368: Error: symbol `.loop_2' is already defined
>> /tmp/cccEyiO7.s:374: Error: symbol `.loop_3' is already defined
>> make[4]: *** [drivers/scsi/arm/arxescsi.o] Error 1

>> It is caused by multiple inline of arxescsi_pseudo_dma_write()
>> which is responsible for the above labels.  Marking the fcn
>> as non-inline fixes the issue.

>> Signed-off-by: Paul Gortmaker<paul.gortmaker@windriver.com>

> Is that a compiler or a code issue? If it's the compiler please fix
> that. If it's the code, then please document why you added the noinline.

> Having said that, "my" compiler compiles drivers/scsi/arm/arxescsi.o
> just fine (using rpc_defconfig on v3.2-rc2).

>> diff --git a/drivers/scsi/arm/arxescsi.c b/drivers/scsi/arm/arxescsi.c
>> index a750aa7..2608a9e 100644
>> --- a/drivers/scsi/arm/arxescsi.c
>> +++ b/drivers/scsi/arm/arxescsi.c
>> @@ -72,7 +72,8 @@ arxescsi_dma_setup(struct Scsi_Host *host, struct scsi_pointer *SCp,
>>   	return fasdma_pseudo;
>>   }
>>
>> -static void arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
>> +static noinline void
>> +arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)

> Shouldn't this line get some indention?

    No, usually such lines don't get indented.

WBR, Sergei


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm: prevent inlining in arxescsi.c causing build failures
  2011-12-01  4:26 Paul Gortmaker
@ 2011-12-01  8:06 ` Uwe Kleine-König
  2011-12-01 10:46   ` Sergei Shtylyov
  2011-12-01 14:06   ` Arnd Bergmann
  0 siblings, 2 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2011-12-01  8:06 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: JBottomley, linux-arm-kernel, linux-scsi, linux-kernel

On Wed, Nov 30, 2011 at 11:26:51PM -0500, Paul Gortmaker wrote:
> The following failure is seen in routine coverage builds:
> 
>   CC [M]  drivers/scsi/arm/arxescsi.o
> /tmp/cccEyiO7.s: Assembler messages:
> /tmp/cccEyiO7.s:334: Error: symbol `.loop_1' is already defined
> /tmp/cccEyiO7.s:337: Error: symbol `.loop_2' is already defined
> /tmp/cccEyiO7.s:343: Error: symbol `.loop_3' is already defined
> /tmp/cccEyiO7.s:365: Error: symbol `.loop_1' is already defined
> /tmp/cccEyiO7.s:368: Error: symbol `.loop_2' is already defined
> /tmp/cccEyiO7.s:374: Error: symbol `.loop_3' is already defined
> make[4]: *** [drivers/scsi/arm/arxescsi.o] Error 1
> 
> It is caused by multiple inline of arxescsi_pseudo_dma_write()
> which is responsible for the above labels.  Marking the fcn
> as non-inline fixes the issue.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Is that a compiler or a code issue? If it's the compiler please fix
that. If it's the code, then please document why you added the noinline.

Having said that, "my" compiler compiles drivers/scsi/arm/arxescsi.o
just fine (using rpc_defconfig on v3.2-rc2).

> diff --git a/drivers/scsi/arm/arxescsi.c b/drivers/scsi/arm/arxescsi.c
> index a750aa7..2608a9e 100644
> --- a/drivers/scsi/arm/arxescsi.c
> +++ b/drivers/scsi/arm/arxescsi.c
> @@ -72,7 +72,8 @@ arxescsi_dma_setup(struct Scsi_Host *host, struct scsi_pointer *SCp,
>  	return fasdma_pseudo;
>  }
>  
> -static void arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
> +static noinline void
> +arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
Shouldn't this line get some indention?

>  {
>         __asm__ __volatile__(
>         "               stmdb   sp!, {r0-r12}\n"

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] arm: prevent inlining in arxescsi.c causing build failures
@ 2011-12-01  4:26 Paul Gortmaker
  2011-12-01  8:06 ` Uwe Kleine-König
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Gortmaker @ 2011-12-01  4:26 UTC (permalink / raw)
  To: JBottomley, linux-arm-kernel, linux-scsi; +Cc: linux-kernel

The following failure is seen in routine coverage builds:

  CC [M]  drivers/scsi/arm/arxescsi.o
/tmp/cccEyiO7.s: Assembler messages:
/tmp/cccEyiO7.s:334: Error: symbol `.loop_1' is already defined
/tmp/cccEyiO7.s:337: Error: symbol `.loop_2' is already defined
/tmp/cccEyiO7.s:343: Error: symbol `.loop_3' is already defined
/tmp/cccEyiO7.s:365: Error: symbol `.loop_1' is already defined
/tmp/cccEyiO7.s:368: Error: symbol `.loop_2' is already defined
/tmp/cccEyiO7.s:374: Error: symbol `.loop_3' is already defined
make[4]: *** [drivers/scsi/arm/arxescsi.o] Error 1

It is caused by multiple inline of arxescsi_pseudo_dma_write()
which is responsible for the above labels.  Marking the fcn
as non-inline fixes the issue.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

diff --git a/drivers/scsi/arm/arxescsi.c b/drivers/scsi/arm/arxescsi.c
index a750aa7..2608a9e 100644
--- a/drivers/scsi/arm/arxescsi.c
+++ b/drivers/scsi/arm/arxescsi.c
@@ -72,7 +72,8 @@ arxescsi_dma_setup(struct Scsi_Host *host, struct scsi_pointer *SCp,
 	return fasdma_pseudo;
 }
 
-static void arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
+static noinline void
+arxescsi_pseudo_dma_write(unsigned char *addr, void __iomem *base)
 {
        __asm__ __volatile__(
        "               stmdb   sp!, {r0-r12}\n"
-- 
1.7.7.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-01-22 19:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-22 19:16 [PATCH] arm: prevent inlining in arxescsi.c causing build failures Paul Gortmaker
  -- strict thread matches above, loose matches on Subject: below --
2011-12-01  4:26 Paul Gortmaker
2011-12-01  8:06 ` Uwe Kleine-König
2011-12-01 10:46   ` Sergei Shtylyov
2011-12-01 14:06   ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).