Re: Latest git oopses during boot

Re: Latest git oopses during boot

[Andrew Morton]

(cc's restored, and expanded a bit)

On Thu, 7 Feb 2008 10:44:29 +0100 "Harald Arnesen" <skogtun.linux@gmail.com> wrote:

> On Feb 6, 2008 11:47 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Wed, 6 Feb 2008 17:32:22 +0100
> > "Harald Arnesen" <skogtun.linux@gmail.com> wrote:
> >
> > > Photo of screen attached (no serial terminal, and alas, no decent
> > > tripod either).
> >
> > Thanks, but you've disabled so many debug options that the trace isn't
> > very useful.
> >
> >
> > > git-bisect gives me this:
> > >
> > > 6b7b651055221127304a4e373ee9b762398d54d7 is first bad commit
> > > commit 6b7b651055221127304a4e373ee9b762398d54d7
> > > Author: FUJITA Tomonori <tomof@acm.org>
> > > Date:   Mon Feb 4 22:27:55 2008 -0800
> > >
> > >     iommu sg merging: add device_dma_parameters structure
> > >
> > >     IOMMUs merges scatter/gather segments without considering a low level
> > >     driver's restrictions. The problem is that IOMMUs can't access to the
> > >     limitations because they are in request_queue.
> > >
> > >     This patchset introduces a new structure, device_dma_parameters,
> > >     including dma information. A pointer to device_dma_parameters is added
> > >     to struct device. The bus specific structures (like pci_dev) includes
> > >     device_dma_parameters. Low level drivers can use dma_set_max_seg_size
> > >     to tell IOMMUs about the restrictions.
> > >
> > >     We can move more dma stuff in struct device (like dma_mask) to struct
> > >     device_dma_parameters later (needs some cleanups before that).
> > >
> > >     This includes patches for all the IOMMUs that could merge sg (x86_64,
> > >     ppc, IA64, alpha, sparc64, and parisc) though only the ppc patch was
> > >     tested. The patches for other IOMMUs are only compile tested.
> > >
> > > $ gcc -v
> > > Using built-in specs.
> > > Target: i686-pc-linux-gnu
> > > Configured with: ../configure --prefix=/opt/gcc
> > > Thread model: posix
> > > gcc version 4.2.3
> > >
> > > Config:
> > >
> > > ...
> > >
> > > # CONFIG_DEBUG_BUGVERBOSE is not set
> > >
> >
> > This one really should be enabled at all times, please.
> >
> > Can you please set it and retry?
> >
> 
> I'm off to my day job now, but I did it earlier this morning. New
> screenshot attached.
> 
> Seems to be the advansys driver, so I tried to remove it - and indeed,
> the kernel now boots. So I guess it's either that driver or my ancient
> Nikon Coolscan II that is the only thing attached to the board.

Thanks.  I uploaded the oops picture to
http://userweb.kernel.org/~akpm/oops.jpg

> Cc to the Matthew Wilcox added.

mm...  looks like all Matthew's changes were in 2.6.23.  And 2.6.23 worked
OK, yes?


The only recent changes to drivers/scsi/advansys.c are 

commit b80ca4f7ee36c26d300c5a8f429e73372d153379
Author: FUJITA Tomonori <tomof@acm.org>
Date:   Sun Jan 13 15:46:13 2008 +0900

    [SCSI] replace sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE
    
    This replaces sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE in
    several LLDs. It's a preparation for the future changes to remove
    sense_buffer array in scsi_cmnd structure.
    
    Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
    Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

:100644 100644 9dd3952... 492702b... M  drivers/scsi/advansys.c

commit 747d016e7e25e216b31022fe2b012508d99fb682
Author: Randy Dunlap <randy.dunlap@oracle.com>
Date:   Mon Jan 14 00:55:18 2008 -0800

    advansys: fix section mismatch warning
    
    Fix section mismatch warning:
    
    WARNING: vmlinux.o(.exit.text+0x152a): Section mismatch: reference to .init.
    
    Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
    Cc: Matthew Wilcox <willy@debian.org>
    Cc: James Bottomley <James.Bottomley@steeleye.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

which seem fairly benign.


gcc inlining is going to make it rather a lot of work to find out which
statement has actually oopsed there.

Re: Latest git oopses during boot

[Harald Arnesen]

On 2/7/08, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> (cc's restored, and expanded a bit)

Ah, sorry, not used to gmail's web interface. Clicked the wrong button.

> > Seems to be the advansys driver, so I tried to remove it - and indeed,
> > the kernel now boots. So I guess it's either that driver or my ancient
> > Nikon Coolscan II that is the only thing attached to the board.
>
> Thanks.  I uploaded the oops picture to
> http://userweb.kernel.org/~akpm/oops.jpg
>
> > Cc to the Matthew Wilcox added.
>
> mm...  looks like all Matthew's changes were in 2.6.23.  And 2.6.23 worked
> OK, yes?

Both 2.6.23 and 2.6.24 are ok.

> The only recent changes to drivers/scsi/advansys.c are
>
> commit b80ca4f7ee36c26d300c5a8f429e73372d153379
> Author: FUJITA Tomonori <tomof@acm.org>
> Date:   Sun Jan 13 15:46:13 2008 +0900
>
>     [SCSI] replace sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE
>
>     This replaces sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE in
>     several LLDs. It's a preparation for the future changes to remove
>     sense_buffer array in scsi_cmnd structure.
>
>     Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
>     Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> :100644 100644 9dd3952... 492702b... M  drivers/scsi/advansys.c
>
> commit 747d016e7e25e216b31022fe2b012508d99fb682
> Author: Randy Dunlap <randy.dunlap@oracle.com>
> Date:   Mon Jan 14 00:55:18 2008 -0800
>
>     advansys: fix section mismatch warning
>
>     Fix section mismatch warning:
>
>     WARNING: vmlinux.o(.exit.text+0x152a): Section mismatch: reference to .init.
>
>     Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>     Cc: Matthew Wilcox <willy@debian.org>
>     Cc: James Bottomley <James.Bottomley@steeleye.com>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>
> which seem fairly benign.
>
>
> gcc inlining is going to make it rather a lot of work to find out which
> statement has actually oopsed there.
-- 
Hilsen Harald

Re: Latest git oopses during boot

[Christoph Hellwig]

On Thu, Feb 07, 2008 at 12:14:56PM +0100, Harald Arnesen wrote:
> > The only recent changes to drivers/scsi/advansys.c are
> >
> > commit b80ca4f7ee36c26d300c5a8f429e73372d153379
> > Author: FUJITA Tomonori <tomof@acm.org>
> > Date:   Sun Jan 13 15:46:13 2008 +0900
> >
> >     [SCSI] replace sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE
> >
> >     This replaces sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE in
> >     several LLDs. It's a preparation for the future changes to remove
> >     sense_buffer array in scsi_cmnd structure.
> >
> >     Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> >     Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

The sense buffer changes have cause a fair amount of trouble, so I'd
look into this one for debugging the problem..

Re: Latest git oopses during boot

[FUJITA Tomonori]

On Thu, 7 Feb 2008 12:14:56 +0100
"Harald Arnesen" <skogtun.linux@gmail.com> wrote:

> On 2/7/08, Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > (cc's restored, and expanded a bit)
> 
> Ah, sorry, not used to gmail's web interface. Clicked the wrong button.
> 
> > > Seems to be the advansys driver, so I tried to remove it - and indeed,
> > > the kernel now boots. So I guess it's either that driver or my ancient
> > > Nikon Coolscan II that is the only thing attached to the board.
> >
> > Thanks.  I uploaded the oops picture to
> > http://userweb.kernel.org/~akpm/oops.jpg
> >
> > > Cc to the Matthew Wilcox added.
> >
> > mm...  looks like all Matthew's changes were in 2.6.23.  And 2.6.23 worked
> > OK, yes?
> 
> Both 2.6.23 and 2.6.24 are ok.
> 
> > The only recent changes to drivers/scsi/advansys.c are
> >
> > commit b80ca4f7ee36c26d300c5a8f429e73372d153379
> > Author: FUJITA Tomonori <tomof@acm.org>
> > Date:   Sun Jan 13 15:46:13 2008 +0900
> >
> >     [SCSI] replace sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE
> >
> >     This replaces sizeof sense_buffer with SCSI_SENSE_BUFFERSIZE in
> >     several LLDs. It's a preparation for the future changes to remove
> >     sense_buffer array in scsi_cmnd structure.
> >
> >     Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> >     Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> >
> > :100644 100644 9dd3952... 492702b... M  drivers/scsi/advansys.c
> >
> > commit 747d016e7e25e216b31022fe2b012508d99fb682
> > Author: Randy Dunlap <randy.dunlap@oracle.com>
> > Date:   Mon Jan 14 00:55:18 2008 -0800
> >
> >     advansys: fix section mismatch warning
> >
> >     Fix section mismatch warning:
> >
> >     WARNING: vmlinux.o(.exit.text+0x152a): Section mismatch: reference to .init.
> >
> >     Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> >     Cc: Matthew Wilcox <willy@debian.org>
> >     Cc: James Bottomley <James.Bottomley@steeleye.com>
> >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> >
> > which seem fairly benign.
> >
> >
> > gcc inlining is going to make it rather a lot of work to find out which
> > statement has actually oopsed there.
> -- 

Can you try this?

Thanks,

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 374ed02..f5dde12 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -566,7 +566,7 @@ typedef struct asc_dvc_var {
 	ASC_SCSI_BIT_ID_TYPE unit_not_ready;
 	ASC_SCSI_BIT_ID_TYPE queue_full_or_busy;
 	ASC_SCSI_BIT_ID_TYPE start_motor;
-	uchar overrun_buf[ASC_OVERRUN_BSIZE] __aligned(8);
+	uchar *overrun_buf;
 	dma_addr_t overrun_dma;
 	uchar scsi_reset_wait;
 	uchar chip_no;
@@ -13833,6 +13833,12 @@ static int __devinit advansys_board_found(struct Scsi_Host *shost,
 	 */
 	if (ASC_NARROW_BOARD(boardp)) {
 		ASC_DBG(2, "AscInitAsc1000Driver()\n");
+
+		asc_dvc_varp->overrun_buf = kzalloc(ASC_OVERRUN_BSIZE, GFP_KERNEL);
+		if (!asc_dvc_varp->overrun_buf) {
+			ret = -ENOMEM;
+			goto err_free_wide_mem;
+		}
 		warn_code = AscInitAsc1000Driver(asc_dvc_varp);
 
 		if (warn_code || asc_dvc_varp->err_code) {
@@ -13840,8 +13846,10 @@ static int __devinit advansys_board_found(struct Scsi_Host *shost,
 					"warn 0x%x, error 0x%x\n",
 					asc_dvc_varp->init_state, warn_code,
 					asc_dvc_varp->err_code);
-			if (asc_dvc_varp->err_code)
+			if (asc_dvc_varp->err_code) {
 				ret = -ENODEV;
+				kfree(asc_dvc_varp->overrun_buf);
+			}
 		}
 	} else {
 		if (advansys_wide_init_chip(shost))
@@ -13891,9 +13899,11 @@ static int advansys_release(struct Scsi_Host *shost)
 		free_dma(shost->dma_channel);
 	}
 	if (ASC_NARROW_BOARD(board)) {
+		ASC_DVC_VAR *asc_dvc_varp = &board->dvc_var.asc_dvc_var;
 		dma_unmap_single(board->dev,
 					board->dvc_var.asc_dvc_var.overrun_dma,
 					ASC_OVERRUN_BSIZE, DMA_FROM_DEVICE);
+		kfree(asc_dvc_varp->overrun_buf);
 	} else {
 		iounmap(board->ioremap_addr);
 		advansys_wide_free_mem(board);

Re: Latest git oopses during boot

[Linus Torvalds]

On Thu, 7 Feb 2008, Harald Arnesen wrote:
> 
> OK, tried it. Another screen shot attached
> (I must really get another box to use as a serial terminal).

This oops decodes to

     8b 44 24 10             mov    0x10(%esp),%eax
     8b 90 7c 02 00 00       mov    0x27c(%eax),%edx
     83 ea 54                sub    $0x54,%edx
     24 18                   and    $0x18,%al
     8b 4c 24 14             mov    0x14(%esp),%ecx
     f6 41 04 04             testb  $0x4,0x4(%ecx)
     75 57                   jne    0x70
     ba d0 80 00 00          mov    $0x80d0,%edx
     b8 68 bf 30 c0          mov    $0xc030bf68,%eax
     e8 2f 8a 38 c7          call   0xc7388a57
**   a3 14 00 00 00          mov    %eax,0x14	**
     85 c0                   test   %eax,%eax
     0f 84 b3 14 00 00       je     0x14c0
     8b 44 24 14             mov    0x14(%esp),%eax
     83 c0 0c                add    $0xc,%eax

and the oopsing instruction is literally an insane "store to absolute
address 0x14" which will definitely oops unconditionally. 

Quit frankly, that code makes no sense.  It smells like code corruption,
especially as it is right at a return point of a function call (ie maybe
the function screwed up the stack accesses somehow).

Actually, it look slike the call address itself is screwed up too.  I
don't think "0xc7388a57" is likely to be a valid address.

The code *looks* like the test

	if (ASC_NARROW_BOARD(boardp)) {
		ASC_DBG(1, "narrow board\n");
		asc_dvc_varp = &boardp->dvc_var.asc_dvc_var;
		asc_dvc_varp->bus_type = bus_type;

but with strange corruption. 

Can you do a

	make drivers/scsi/advansys.lst

and see what it should be?

		Linus

Re: Latest git oopses during boot

[Linus Torvalds]

On Thu, 7 Feb 2008, Harald Arnesen wrote:
> >
> > Can you do a
> >
> >         make drivers/scsi/advansys.lst
> >
> > and see what it should be?
> 
> Anyway, here it is, as an attachment.

Ok, I was wrong. The code really *does* compile to that insane

	a3 14 00 00 00          mov    %eax,0x14

by your compiler.

That's the

	asc_dvc_varp->overrun_buf = kzalloc(ASC_OVERRUN_BSIZE, GFP_KERNEL);

thing, and gcc seems to have decided that it can statically prove that 
asc_dvc_varp is NULL.

Quite frankly, I don't see that being true. But you have some patches in 
your tree that I haven't followed, so.. Are you sure the patches applied 
to the right spot? The patch I saw added that kzalloc() to the _end_ of 
the function (long after asc_dvc_varp was initialized), maybe that one got 
mis-applied?

Or maybe your compiler version is simply totally broken.

			Linus

Re: Latest git oopses during boot

[Harald Arnesen]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, 7 Feb 2008, Harald Arnesen wrote:
>> >
>> > Can you do a
>> >
>> >         make drivers/scsi/advansys.lst
>> >
>> > and see what it should be?
>> 
>> Anyway, here it is, as an attachment.
>
> Ok, I was wrong. The code really *does* compile to that insane
>
> 	a3 14 00 00 00          mov    %eax,0x14
>
> by your compiler.
>
> That's the
>
> 	asc_dvc_varp->overrun_buf = kzalloc(ASC_OVERRUN_BSIZE, GFP_KERNEL);
>
> thing, and gcc seems to have decided that it can statically prove that 
> asc_dvc_varp is NULL.
>
> Quite frankly, I don't see that being true. But you have some patches in 
> your tree that I haven't followed, so.. Are you sure the patches applied 
> to the right spot? The patch I saw added that kzalloc() to the _end_ of 
> the function (long after asc_dvc_varp was initialized), maybe that one got 
> mis-applied?
>
> Or maybe your compiler version is simply totally broken.
>
> 			Linus

I'll try applying the patch to a freshly downloaded git-tree.

Shall I try another compiler? I have at least these two:

gcc version 3.4.6 (Ubuntu 3.4.6-6ubuntu2)
gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)

in addition to the self-compiled 4.2.3 I used for the tests.
-- 
Hilsen Harald.

Re: Latest git oopses during boot

[Linus Torvalds]

On Thu, 7 Feb 2008, Harald Arnesen wrote:
> 
> I'll try applying the patch to a freshly downloaded git-tree.

Ok, good.

> Shall I try another compiler? I have at least these two:
> 
> gcc version 3.4.6 (Ubuntu 3.4.6-6ubuntu2)
> gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)

I would suggest a patch mis-application problem first (or possibly even 
the patch itself being broken - I simply didn't look very closely at the 
patch, but it *looked* ok).

If it's a compiler bug, it's a pretty big one, and quite frankly, I doubt 
it. Compiler bugs do happen, but they are pretty rare, and they tend to 
have more subtle effects than the one you see. 

However:

> in addition to the self-compiled 4.2.3 I used for the tests.

4.2.3? Really? That's pretty damn recent, and so almost totally untested. 
That does make a compiler bug at least more likely.

So yes, if you already have other compilers installed, you should try 
them. If it really is a compiler bug, it's a really bad one, and you would 
want to let the gcc people know.

Still, I'd double-check that the

	asc_dvc_varp->overrun_buf = kzalloc(ASC_OVERRUN_BSIZE, GFP_KERNEL);

line was added properly first. You should see it way after the point where 
it did 

	asc_dvc_varp = &boardp->dvc_var.asc_dvc_var;

to initialize it (and both statements should be inside a

	if (ASC_NARROW_BOARD(boardp)) {

conditional - please check that the source code looks sane too).

			Linus

Re: Latest git oopses during boot

[Harald Arnesen]

On 2/7/08, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Thu, 7 Feb 2008, Harald Arnesen wrote:
> >
> > I'll try applying the patch to a freshly downloaded git-tree.
>
> Ok, good.
>
> > Shall I try another compiler? I have at least these two:
> >
> > gcc version 3.4.6 (Ubuntu 3.4.6-6ubuntu2)
> > gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)
>
> I would suggest a patch mis-application problem first (or possibly even
> the patch itself being broken - I simply didn't look very closely at the
> patch, but it *looked* ok).
>
> If it's a compiler bug, it's a pretty big one, and quite frankly, I doubt
> it. Compiler bugs do happen, but they are pretty rare, and they tend to
> have more subtle effects than the one you see.
>
> However:
>
> > in addition to the self-compiled 4.2.3 I used for the tests.
>
> 4.2.3? Really? That's pretty damn recent, and so almost totally untested.
> That does make a compiler bug at least more likely.
>
> So yes, if you already have other compilers installed, you should try
> them. If it really is a compiler bug, it's a really bad one, and you would
> want to let the gcc people know.
>
> Still, I'd double-check that the
>
>         asc_dvc_varp->overrun_buf = kzalloc(ASC_OVERRUN_BSIZE, GFP_KERNEL);
>
> line was added properly first. You should see it way after the point where
> it did
>
>         asc_dvc_varp = &boardp->dvc_var.asc_dvc_var;
>
> to initialize it (and both statements should be inside a
>
>         if (ASC_NARROW_BOARD(boardp)) {
>
> conditional - please check that the source code looks sane too).
>
>                         Linus

I just re-downloaded an re-patched and re-compiled (with gcc 4.2.3),
and now the kernel boots. I must have screwed up the previous
patching.

It now works, with Fujita's patch applied.
-- 
Hilsen Harald

Re: Latest git oopses during boot

[FUJITA Tomonori]

On Thu, 7 Feb 2008 23:24:00 +0100
"Harald Arnesen" <skogtun.linux@gmail.com> wrote:

> On 2/7/08, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> >
> > On Thu, 7 Feb 2008, Harald Arnesen wrote:
> > >
> > > I'll try applying the patch to a freshly downloaded git-tree.
> >
> > Ok, good.
> >
> > > Shall I try another compiler? I have at least these two:
> > >
> > > gcc version 3.4.6 (Ubuntu 3.4.6-6ubuntu2)
> > > gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)
> >
> > I would suggest a patch mis-application problem first (or possibly even
> > the patch itself being broken - I simply didn't look very closely at the
> > patch, but it *looked* ok).
> >
> > If it's a compiler bug, it's a pretty big one, and quite frankly, I doubt
> > it. Compiler bugs do happen, but they are pretty rare, and they tend to
> > have more subtle effects than the one you see.
> >
> > However:
> >
> > > in addition to the self-compiled 4.2.3 I used for the tests.
> >
> > 4.2.3? Really? That's pretty damn recent, and so almost totally untested.
> > That does make a compiler bug at least more likely.
> >
> > So yes, if you already have other compilers installed, you should try
> > them. If it really is a compiler bug, it's a really bad one, and you would
> > want to let the gcc people know.
> >
> > Still, I'd double-check that the
> >
> >         asc_dvc_varp->overrun_buf = kzalloc(ASC_OVERRUN_BSIZE, GFP_KERNEL);
> >
> > line was added properly first. You should see it way after the point where
> > it did
> >
> >         asc_dvc_varp = &boardp->dvc_var.asc_dvc_var;
> >
> > to initialize it (and both statements should be inside a
> >
> >         if (ASC_NARROW_BOARD(boardp)) {
> >
> > conditional - please check that the source code looks sane too).
> >
> >                         Linus
> 
> I just re-downloaded an re-patched and re-compiled (with gcc 4.2.3),
> and now the kernel boots. I must have screwed up the previous
> patching.
> 
> It now works, with Fujita's patch applied.

Thanks Harald and Linus,

The bug has been in the advansys driver. 2.6.23 and 2.6.24 works just
because the size of Scsi_Host structure was multiples of 8. After
2.6.24, some patches change Scsi_Host structure and now the size is
not multiples of 8. So we hit this bug.


I'll resend the patch with a proper description.