2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Torsten Kaiser]

Both 2.6.24-rc1 and the current git version fail to boot for me:

[   57.182205] ohci1394: fw-host0: OHCI-1394 1.1 (PCI): IRQ=[16]
MMIO=[effff800-efffffff]  Max Packet=
[2048]  IR/IT contexts=[4/8]
[   57.194032] eth1394: eth2: IPv4 over IEEE 1394 (fw-host0)
[   57.199527] ------------[ cut here ]------------
[   57.204154] kernel BUG at include/linux/scatterlist.h:49!
[   57.209559] invalid opcode: 0000 [1] SMP
[   57.213667] CPU 0
[   57.215742] Modules linked in:
[   57.218857] Pid: 1, comm: swapper Not tainted 2.6.24-rc1 #1
[   57.224435] RIP: 0010:[<ffffffff80463512>]  [<ffffffff80463512>]
dma_region_alloc+0x142/0x180
[   57.233003] RSP: 0000:ffff810004f29db0  EFLAGS: 00010293
[   57.238322] RAX: ffffc20001454000 RBX: 0000000000000000 RCX: ffffe20001c7e690
[   57.245457] RDX: 0000000000000000 RSI: 00003ffffffff000 RDI: 0000000000000388
[   57.252592] RBP: ffff810005ddca48 R08: 0000000000000000 R09: ffffc20001454000
[   57.259727] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000087654321
[   57.266862] R13: ffff810100914000 R14: 0000000000000002 R15: ffff810005e88000
[   57.273996] FS:  0000000000000000(0000) GS:ffffffff8079c000(0000)
knlGS:0000000000000000
[   57.282082] CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
[   57.287834] CR2: 0000000000e01766 CR3: 0000000000201000 CR4: 00000000000006e0
[   57.294968] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   57.302102] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   57.309238] Process swapper (pid: 1, threadinfo ffff810004f28000,
task ffff81010032a000)
[   57.317332] Stack:  ffff810005ddca48 0000000000000002
ffff810005ddca00 0000000000000010
[   57.325546]  000000000000001f ffffffff80464146 ffffffff8089ff00
0000000000010000
[   57.333104]  ffff810005ddca00 ffff8101011b66c0 ffff810005e88000
ffffffff80754740
[   57.340455] Call Trace:
[   57.343139]  [<ffffffff80464146>] hpsb_iso_common_init+0x1e6/0x210
[   57.349322]  [<ffffffff80464198>] hpsb_iso_recv_init+0x28/0x70
[   57.355159]  [<ffffffff8046c940>] ether1394_iso+0x0/0x130
[   57.360564]  [<ffffffff8046bc08>] ether1394_recv_init+0x58/0xd0
[   57.366488]  [<ffffffff8046c940>] ether1394_iso+0x0/0x130
[   57.371893]  [<ffffffff8045e8b7>] highlevel_for_each_host_reg+0x17/0x60
[   57.378508]  [<ffffffff8045e8a0>] highlevel_for_each_host_reg+0x0/0x60
[   57.385039]  [<ffffffff80460f74>] nodemgr_for_each_host+0x44/0x90
[   57.391136]  [<ffffffff807dfac6>] ether1394_init_module+0x36/0x70
[   57.397232]  [<ffffffff807be724>] kernel_init+0x164/0x350
[   57.402637]  [<ffffffff8020c9c8>] child_rip+0xa/0x12
[   57.407609]  [<ffffffff807be5c0>] kernel_init+0x0/0x350
[   57.412842]  [<ffffffff8020c9be>] child_rip+0x0/0x12
[   57.417814]
[   57.419327]
[   57.419327] Code: 0f 0b eb fe 0f 0b eb fe 66 0f 1f 44 00 00 48 c7 c7 38 40 6e
[   57.428891] RIP  [<ffffffff80463512>] dma_region_alloc+0x142/0x180
[   57.435117]  RSP <ffff810004f29db0>
[   57.438629] Kernel panic - not syncing: Attempted to kill init!

from gdb:
0xffffffff80463512 is in dma_region_alloc (include/linux/scatterlist.h:49).
44               * In order for the low bit stealing approach to work, pages
45               * must be aligned at a 32-bit boundary as a minimum.
46               */
47              BUG_ON((unsigned long) page & 0x03);
48      #ifdef CONFIG_DEBUG_SG
49              BUG_ON(sg->sg_magic != SG_MAGIC);
50      #endif
51              sg->page_link = page_link | (unsigned long) page;
52      }
53

Torsten

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Stefan Richter]

Torsten Kaiser wrote:
> Both 2.6.24-rc1 and the current git version fail to boot for me:
> 
> [   57.182205] ohci1394: fw-host0: OHCI-1394 1.1 (PCI): IRQ=[16]
> MMIO=[effff800-efffffff]  Max Packet=
> [2048]  IR/IT contexts=[4/8]
> [   57.194032] eth1394: eth2: IPv4 over IEEE 1394 (fw-host0)
> [   57.199527] ------------[ cut here ]------------
> [   57.204154] kernel BUG at include/linux/scatterlist.h:49!

To which extent do you need IEEE 1394 drivers?

[...]
> from gdb:
> 0xffffffff80463512 is in dma_region_alloc (include/linux/scatterlist.h:49).
> 44               * In order for the low bit stealing approach to work, pages
> 45               * must be aligned at a 32-bit boundary as a minimum.
> 46               */
> 47              BUG_ON((unsigned long) page & 0x03);
> 48      #ifdef CONFIG_DEBUG_SG
> 49              BUG_ON(sg->sg_magic != SG_MAGIC);
> 50      #endif
> 51              sg->page_link = page_link | (unsigned long) page;
> 52      }
> 53

Uh oh...

I'm afraid the ieee1394 core's own scatter-gather list managing code
didn't survive the changes in the kernel's s/g list implementation.

Alas I will be quite busy with non-Linux related stuff during at least
the next two weeks and can't fix this myself for now.
-- 
Stefan Richter
-=====-=-=== =-== ---=-
http://arcgraph.de/sr/

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Stefan Richter]

I wrote:
> Torsten Kaiser wrote:
>> Both 2.6.24-rc1 and the current git version fail to boot for me:
>>
>> [   57.182205] ohci1394: fw-host0: OHCI-1394 1.1 (PCI): IRQ=[16] MMIO=[effff800-efffffff]  Max Packet=[2048]  IR/IT contexts=[4/8]
>> [   57.194032] eth1394: eth2: IPv4 over IEEE 1394 (fw-host0)
>> [   57.199527] ------------[ cut here ]------------
>> [   57.204154] kernel BUG at include/linux/scatterlist.h:49!
>> [   57.209559] invalid opcode: 0000 [1] SMP
>> [   57.213667] CPU 0
>> [   57.215742] Modules linked in:
>> [   57.218857] Pid: 1, comm: swapper Not tainted 2.6.24-rc1 #1
>> [   57.224435] RIP: 0010:[<ffffffff80463512>]  [<ffffffff80463512>] dma_region_alloc+0x142/0x180
>> [   57.233003] RSP: 0000:ffff810004f29db0  EFLAGS: 00010293
>> [   57.238322] RAX: ffffc20001454000 RBX: 0000000000000000 RCX: ffffe20001c7e690
>> [   57.245457] RDX: 0000000000000000 RSI: 00003ffffffff000 RDI: 0000000000000388
>> [   57.252592] RBP: ffff810005ddca48 R08: 0000000000000000 R09: ffffc20001454000
>> [   57.259727] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000087654321
>> [   57.266862] R13: ffff810100914000 R14: 0000000000000002 R15: ffff810005e88000
>> [   57.273996] FS:  0000000000000000(0000) GS:ffffffff8079c000(0000) knlGS:0000000000000000
>> [   57.282082] CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
>> [   57.287834] CR2: 0000000000e01766 CR3: 0000000000201000 CR4: 00000000000006e0
>> [   57.294968] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [   57.302102] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>> [   57.309238] Process swapper (pid: 1, threadinfo ffff810004f28000, task ffff81010032a000)
>> [   57.317332] Stack:  ffff810005ddca48 0000000000000002 ffff810005ddca00 0000000000000010
>> [   57.325546]  000000000000001f ffffffff80464146 ffffffff8089ff00 0000000000010000
>> [   57.333104]  ffff810005ddca00 ffff8101011b66c0 ffff810005e88000 ffffffff80754740
>> [   57.340455] Call Trace:
>> [   57.343139]  [<ffffffff80464146>] hpsb_iso_common_init+0x1e6/0x210
>> [   57.349322]  [<ffffffff80464198>] hpsb_iso_recv_init+0x28/0x70
>> [   57.355159]  [<ffffffff8046c940>] ether1394_iso+0x0/0x130
>> [   57.360564]  [<ffffffff8046bc08>] ether1394_recv_init+0x58/0xd0
[...]
>> from gdb:
>> 0xffffffff80463512 is in dma_region_alloc (include/linux/scatterlist.h:49).
>> 44               * In order for the low bit stealing approach to work, pages
>> 45               * must be aligned at a 32-bit boundary as a minimum.
>> 46               */
>> 47              BUG_ON((unsigned long) page & 0x03);
>> 48      #ifdef CONFIG_DEBUG_SG
>> 49              BUG_ON(sg->sg_magic != SG_MAGIC);
>> 50      #endif
>> 51              sg->page_link = page_link | (unsigned long) page;
>> 52      }
>> 53
> 
> Uh oh...
> 
> I'm afraid the ieee1394 core's own scatter-gather list managing code
> didn't survive the changes in the kernel's s/g list implementation.
> 
> Alas I will be quite busy with non-Linux related stuff during at least
> the next two weeks and can't fix this myself for now.

Jens, maybe you can come up with a fix quicker than I could.
-- 
Stefan Richter
-=====-=-=== =-== ---=-
http://arcgraph.de/sr/

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Jens Axboe]

On Sat, Nov 03 2007, Stefan Richter wrote:
> I wrote:
> > Torsten Kaiser wrote:
> >> Both 2.6.24-rc1 and the current git version fail to boot for me:
> >>
> >> [   57.182205] ohci1394: fw-host0: OHCI-1394 1.1 (PCI): IRQ=[16] MMIO=[effff800-efffffff]  Max Packet=[2048]  IR/IT contexts=[4/8]
> >> [   57.194032] eth1394: eth2: IPv4 over IEEE 1394 (fw-host0)
> >> [   57.199527] ------------[ cut here ]------------
> >> [   57.204154] kernel BUG at include/linux/scatterlist.h:49!
> >> [   57.209559] invalid opcode: 0000 [1] SMP
> >> [   57.213667] CPU 0
> >> [   57.215742] Modules linked in:
> >> [   57.218857] Pid: 1, comm: swapper Not tainted 2.6.24-rc1 #1
> >> [   57.224435] RIP: 0010:[<ffffffff80463512>]  [<ffffffff80463512>] dma_region_alloc+0x142/0x180
> >> [   57.233003] RSP: 0000:ffff810004f29db0  EFLAGS: 00010293
> >> [   57.238322] RAX: ffffc20001454000 RBX: 0000000000000000 RCX: ffffe20001c7e690
> >> [   57.245457] RDX: 0000000000000000 RSI: 00003ffffffff000 RDI: 0000000000000388
> >> [   57.252592] RBP: ffff810005ddca48 R08: 0000000000000000 R09: ffffc20001454000
> >> [   57.259727] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000087654321
> >> [   57.266862] R13: ffff810100914000 R14: 0000000000000002 R15: ffff810005e88000
> >> [   57.273996] FS:  0000000000000000(0000) GS:ffffffff8079c000(0000) knlGS:0000000000000000
> >> [   57.282082] CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
> >> [   57.287834] CR2: 0000000000e01766 CR3: 0000000000201000 CR4: 00000000000006e0
> >> [   57.294968] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> >> [   57.302102] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> >> [   57.309238] Process swapper (pid: 1, threadinfo ffff810004f28000, task ffff81010032a000)
> >> [   57.317332] Stack:  ffff810005ddca48 0000000000000002 ffff810005ddca00 0000000000000010
> >> [   57.325546]  000000000000001f ffffffff80464146 ffffffff8089ff00 0000000000010000
> >> [   57.333104]  ffff810005ddca00 ffff8101011b66c0 ffff810005e88000 ffffffff80754740
> >> [   57.340455] Call Trace:
> >> [   57.343139]  [<ffffffff80464146>] hpsb_iso_common_init+0x1e6/0x210
> >> [   57.349322]  [<ffffffff80464198>] hpsb_iso_recv_init+0x28/0x70
> >> [   57.355159]  [<ffffffff8046c940>] ether1394_iso+0x0/0x130
> >> [   57.360564]  [<ffffffff8046bc08>] ether1394_recv_init+0x58/0xd0
> [...]
> >> from gdb:
> >> 0xffffffff80463512 is in dma_region_alloc (include/linux/scatterlist.h:49).
> >> 44               * In order for the low bit stealing approach to work, pages
> >> 45               * must be aligned at a 32-bit boundary as a minimum.
> >> 46               */
> >> 47              BUG_ON((unsigned long) page & 0x03);
> >> 48      #ifdef CONFIG_DEBUG_SG
> >> 49              BUG_ON(sg->sg_magic != SG_MAGIC);
> >> 50      #endif
> >> 51              sg->page_link = page_link | (unsigned long) page;
> >> 52      }
> >> 53
> > 
> > Uh oh...
> > 
> > I'm afraid the ieee1394 core's own scatter-gather list managing code
> > didn't survive the changes in the kernel's s/g list implementation.
> > 
> > Alas I will be quite busy with non-Linux related stuff during at least
> > the next two weeks and can't fix this myself for now.
> 
> Jens, maybe you can come up with a fix quicker than I could.

Sure, I'll take a look!

-- 
Jens Axboe

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Torsten Kaiser]

On 11/2/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Torsten Kaiser wrote:
> > Both 2.6.24-rc1 and the current git version fail to boot for me:
> >
> > [   57.182205] ohci1394: fw-host0: OHCI-1394 1.1 (PCI): IRQ=[16]
> > MMIO=[effff800-efffffff]  Max Packet=
> > [2048]  IR/IT contexts=[4/8]
> > [   57.194032] eth1394: eth2: IPv4 over IEEE 1394 (fw-host0)
> > [   57.199527] ------------[ cut here ]------------
> > [   57.204154] kernel BUG at include/linux/scatterlist.h:49!
>
> To which extent do you need IEEE 1394 drivers?

Using eth1394 as primary network connection on this computer.
So switching to the new stack is currently not an option, or did I
missing something?
(I also have a firewire disk, but that is not used right now)

> > from gdb:
> > 0xffffffff80463512 is in dma_region_alloc (include/linux/scatterlist.h:49).
> > 44               * In order for the low bit stealing approach to work, pages
> > 45               * must be aligned at a 32-bit boundary as a minimum.
> > 46               */
> > 47              BUG_ON((unsigned long) page & 0x03);
> > 48      #ifdef CONFIG_DEBUG_SG
> > 49              BUG_ON(sg->sg_magic != SG_MAGIC);
> > 50      #endif
> > 51              sg->page_link = page_link | (unsigned long) page;
> > 52      }
> > 53
>
> Uh oh...

Looking that calltrace upwards, it seems replacing the
memset(dma->sglist,...) with sg_init_table(...) would fix the BUG_ON()
as that inits the SG_MAGIC. But I do not trust myself to fixing all
the iterators correctly to convert this completely to the new API.

What I noticed trying to find the definition of sg_dma_address() is
that all architectures seem to implement identical macros for
sg_dma_address() and sg_dma_length(). The only difference is that some
archs call their fields different.

But I stopped looking if unifiying all names was possible when I found
swiotlb_map_sg() in lib/swiotlb.c...

Why is there a comment about using "sg_dma_{address,length}(SG)" to
obtain the "appropriate dma address and length" when I see no instance
of this macros there? Instead there are direct accesses to sg->length
and sg->dma_length. And the field dma_length only exists in some
arches! (alpha, ia64, powerpc only if __powerpc64__ is defined,
sparc64 and the 64bit variant of x86)
On the other hand I can't find any other "iova_length" in the hole
tree apart from include/asm-parisc/scatterlist.h...

... so I can't really make sense of the sg mechanism and abstain from
trying to fix this myself.

Torsten

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Stefan Richter]

Torsten Kaiser wrote:
> On 11/2/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> To which extent do you need IEEE 1394 drivers?
> 
> Using eth1394 as primary network connection on this computer.
> So switching to the new stack is currently not an option,

That's right.

> Looking that calltrace upwards, it seems replacing the
> memset(dma->sglist,...) with sg_init_table(...) would fix the BUG_ON()
> as that inits the SG_MAGIC.

Yes, this should be the first thing to be fixed.

> But I do not trust myself to fixing all
> the iterators correctly to convert this completely to the new API.

Since ieee1394 doesn't create chained s/g lists, we might even get away
with ieee1394's existing iterators.

(The other place where the 1394 stack deals with s/g lists is sbp2 where
we have to deal with what comes from the SCSI stack or the block layer.
But sbp2's s/g list handling is entirely unrelated to ieee1394's own
code for isochronous I/O and async streams, which eth1394 needs and
caused the bug.)
-- 
Stefan Richter
-=====-=-=== =-== ---=-
http://arcgraph.de/sr/

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Jens Axboe]

On Sat, Nov 03 2007, Stefan Richter wrote:
> Torsten Kaiser wrote:
> > On 11/2/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> To which extent do you need IEEE 1394 drivers?
> > 
> > Using eth1394 as primary network connection on this computer.
> > So switching to the new stack is currently not an option,
> 
> That's right.
> 
> > Looking that calltrace upwards, it seems replacing the
> > memset(dma->sglist,...) with sg_init_table(...) would fix the BUG_ON()
> > as that inits the SG_MAGIC.
> 
> Yes, this should be the first thing to be fixed.

It's probably enough. Only if you use chaining do you need to convert to
using for_each_sg() and so on.

-- 
Jens Axboe

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Jens Axboe]

On Sat, Nov 03 2007, Jens Axboe wrote:
> On Sat, Nov 03 2007, Stefan Richter wrote:
> > Torsten Kaiser wrote:
> > > On 11/2/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> > >> To which extent do you need IEEE 1394 drivers?
> > > 
> > > Using eth1394 as primary network connection on this computer.
> > > So switching to the new stack is currently not an option,
> > 
> > That's right.
> > 
> > > Looking that calltrace upwards, it seems replacing the
> > > memset(dma->sglist,...) with sg_init_table(...) would fix the BUG_ON()
> > > as that inits the SG_MAGIC.
> > 
> > Yes, this should be the first thing to be fixed.
> 
> It's probably enough. Only if you use chaining do you need to convert to
> using for_each_sg() and so on.

Did a grep over ieee1394/, this seems to be all you need.

diff --git a/drivers/ieee1394/dma.c b/drivers/ieee1394/dma.c
index f5f4983..7c4eb39 100644
--- a/drivers/ieee1394/dma.c
+++ b/drivers/ieee1394/dma.c
@@ -103,8 +103,7 @@ int dma_region_alloc(struct dma_region *dma, unsigned long n_bytes,
 		goto err;
 	}
 
-	/* just to be safe - this will become unnecessary once sglist->address goes away */
-	memset(dma->sglist, 0, dma->n_pages * sizeof(*dma->sglist));
+	sg_init_table(dma->sglist, dma->n_pages);
 
 	/* fill scatter/gather list with pages */
 	for (i = 0; i < dma->n_pages; i++) {

-- 
Jens Axboe

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Torsten Kaiser]

On 11/4/07, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Sat, Nov 03 2007, Jens Axboe wrote:
> > It's probably enough. Only if you use chaining do you need to convert to
> > using for_each_sg() and so on.
>
> Did a grep over ieee1394/, this seems to be all you need.

What *I* need. For eth1394.

Stefan Richter wrote:
> (The other place where the 1394 stack deals with s/g lists is sbp2 where
> we have to deal with what comes from the SCSI stack or the block layer.
> But sbp2's s/g list handling is entirely unrelated to ieee1394's own
> code for isochronous I/O and async streams, which eth1394 needs and
> caused the bug.)

I see nothing preventing chained sg lists to be feed into
sbp2scsi_queuecommand() which then gives these lists to
sbp2_prep_command_orb_sg().

And there I find this:
static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb,
                                     struct sbp2_fwhost_info *hi,
                                     struct sbp2_command_info *cmd,
                                     unsigned int scsi_use_sg,
                                     struct scatterlist *sgpnt,
                                     u32 orb_direction,
                                     enum dma_data_direction dma_dir)
{
[snip]
for (i = 0, sg_count = 0 ; i < count; i++, sgpnt++) {


As yesterday my md1_raid5-thread oopsed with the same bug from the
thread "kernel NULL pointer dereference in blk_rq_map_sg with
v2.6.23-6815-g0895e91" I'm rather suspicious of anything sg related
right now. (At least I think its the same bug, as 2.6.23-mm1 does not
contain the fix from that thread)

> diff --git a/drivers/ieee1394/dma.c b/drivers/ieee1394/dma.c
> index f5f4983..7c4eb39 100644
> --- a/drivers/ieee1394/dma.c
> +++ b/drivers/ieee1394/dma.c
> @@ -103,8 +103,7 @@ int dma_region_alloc(struct dma_region *dma, unsigned long n_bytes,
>                 goto err;
>         }
>
> -       /* just to be safe - this will become unnecessary once sglist->address goes away */
> -       memset(dma->sglist, 0, dma->n_pages * sizeof(*dma->sglist));
> +       sg_init_table(dma->sglist, dma->n_pages);
>
>         /* fill scatter/gather list with pages */
>         for (i = 0; i < dma->n_pages; i++) {
>

With that patch, 2.6.24-rc1-b4f555081fdd27d13e6ff39d455d5aefae9d2c0c
boots, eth1394 works.

Torsten

[PATCH 1/3 2.6.24-rc1-gitX] ieee1394: iso and async streams: s/g list fix

[Stefan Richter]

Date: Sun, 4 Nov 2007 09:44:56 +0100
From: Jens Axboe <jens.axboe@oracle.com>

Torsten Kaiser wrote:
> Looking that calltrace upwards, it seems replacing the
> memset(dma->sglist,...) with sg_init_table(...) would fix the BUG_ON()
> as that inits the SG_MAGIC.

Tested-by: Torsten Kaiser <just.for.lkml@googlemail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/dma.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.24-rc1/drivers/ieee1394/dma.c
===================================================================
--- linux-2.6.24-rc1.orig/drivers/ieee1394/dma.c
+++ linux-2.6.24-rc1/drivers/ieee1394/dma.c
@@ -103,8 +103,7 @@ int dma_region_alloc(struct dma_region *
 		goto err;
 	}
 
-	/* just to be safe - this will become unnecessary once sglist->address goes away */
-	memset(dma->sglist, 0, dma->n_pages * sizeof(*dma->sglist));
+	sg_init_table(dma->sglist, dma->n_pages);
 
 	/* fill scatter/gather list with pages */
 	for (i = 0; i < dma->n_pages; i++) {

-- 
Stefan Richter
-=====-=-=== =-== --=--
http://arcgraph.de/sr/

[PATCH 2/3 2.6.24-rc1-gitX] ieee1394: sbp2: s/g list fix

[Stefan Richter]

Don't panic on chained s/g lists.  Only compile-tested.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/sbp2.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux/drivers/ieee1394/sbp2.c
===================================================================
--- linux.orig/drivers/ieee1394/sbp2.c
+++ linux/drivers/ieee1394/sbp2.c
@@ -1489,7 +1489,7 @@ static void sbp2_prep_command_orb_sg(str
 
 		/* loop through and fill out our SBP-2 page tables
 		 * (and split up anything too large) */
-		for (i = 0, sg_count = 0 ; i < count; i++, sgpnt++) {
+		for (i = 0, sg_count = 0 ; i < count; i++) {
 			sg_len = sg_dma_len(sgpnt);
 			sg_addr = sg_dma_address(sgpnt);
 			while (sg_len) {
@@ -1506,6 +1506,7 @@ static void sbp2_prep_command_orb_sg(str
 				}
 				sg_count++;
 			}
+			sgpnt = sg_next(sgpnt);
 		}
 
 		orb->misc |= ORB_SET_DATA_SIZE(sg_count);

-- 
Stefan Richter
-=====-=-=== =-== --=--
http://arcgraph.de/sr/

[PATCH 3/3 2.6.24-rc1-gitX] firewire: fw-sbp2: s/g list fix

[Stefan Richter]

Don't panic on chained s/g lists.  Only compile-tested.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-sbp2.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux/drivers/firewire/fw-sbp2.c
===================================================================
--- linux.orig/drivers/firewire/fw-sbp2.c
+++ linux/drivers/firewire/fw-sbp2.c
@@ -1100,9 +1100,9 @@ sbp2_map_scatterlist(struct sbp2_command
 	 * elements larger than 65535 bytes, some IOMMUs may merge sg elements
 	 * during DMA mapping, and Linux currently doesn't prevent this.
 	 */
-	for (i = 0, j = 0; i < count; i++) {
-		sg_len = sg_dma_len(sg + i);
-		sg_addr = sg_dma_address(sg + i);
+	for (i = 0, j = 0; i < count; i++, sg = sg_next(sg)) {
+		sg_len = sg_dma_len(sg);
+		sg_addr = sg_dma_address(sg);
 		while (sg_len) {
 			/* FIXME: This won't get us out of the pinch. */
 			if (unlikely(j >= ARRAY_SIZE(orb->page_table))) {

-- 
Stefan Richter
-=====-=-=== =-== --=--
http://arcgraph.de/sr/

Re: [PATCH 2/3 2.6.24-rc1-gitX] ieee1394: sbp2: s/g list fix

[Stefan Richter]

Don't panic on chained s/g lists.  Only compile-tested.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
Update:
 The loop body doesn't honor the 80 columns limit either.
 Can be cleaned up later by renaming a variable.

 drivers/ieee1394/sbp2.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/ieee1394/sbp2.c
===================================================================
--- linux.orig/drivers/ieee1394/sbp2.c
+++ linux/drivers/ieee1394/sbp2.c
@@ -1489,7 +1489,7 @@ static void sbp2_prep_command_orb_sg(str
 
 		/* loop through and fill out our SBP-2 page tables
 		 * (and split up anything too large) */
-		for (i = 0, sg_count = 0 ; i < count; i++, sgpnt++) {
+		for (i = 0, sg_count = 0; i < count; i++, sgpnt = sg_next(sgpnt)) {
 			sg_len = sg_dma_len(sgpnt);
 			sg_addr = sg_dma_address(sgpnt);
 			while (sg_len) {

-- 
Stefan Richter
-=====-=-=== =-== --=--
http://arcgraph.de/sr/

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Jens Axboe]

On Sun, Nov 04 2007, Torsten Kaiser wrote:
> On 11/4/07, Jens Axboe <jens.axboe@oracle.com> wrote:
> > On Sat, Nov 03 2007, Jens Axboe wrote:
> > > It's probably enough. Only if you use chaining do you need to convert to
> > > using for_each_sg() and so on.
> >
> > Did a grep over ieee1394/, this seems to be all you need.
> 
> What *I* need. For eth1394.
> 
> Stefan Richter wrote:
> > (The other place where the 1394 stack deals with s/g lists is sbp2 where
> > we have to deal with what comes from the SCSI stack or the block layer.
> > But sbp2's s/g list handling is entirely unrelated to ieee1394's own
> > code for isochronous I/O and async streams, which eth1394 needs and
> > caused the bug.)
> 
> I see nothing preventing chained sg lists to be feed into
> sbp2scsi_queuecommand() which then gives these lists to
> sbp2_prep_command_orb_sg().
> 
> And there I find this:
> static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb,
>                                      struct sbp2_fwhost_info *hi,
>                                      struct sbp2_command_info *cmd,
>                                      unsigned int scsi_use_sg,
>                                      struct scatterlist *sgpnt,
>                                      u32 orb_direction,
>                                      enum dma_data_direction dma_dir)
> {
> [snip]
> for (i = 0, sg_count = 0 ; i < count; i++, sgpnt++) {

Chained sg lists will only be feed to a scsi host controller that
enables chaining in its host template.

The fix looks fine though, it's just not a requirement or bug fix :-)

> As yesterday my md1_raid5-thread oopsed with the same bug from the
> thread "kernel NULL pointer dereference in blk_rq_map_sg with
> v2.6.23-6815-g0895e91" I'm rather suspicious of anything sg related
> right now. (At least I think its the same bug, as 2.6.23-mm1 does not
> contain the fix from that thread)

Can you post that oops please?

-- 
Jens Axboe

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Stefan Richter]

Jens Axboe wrote:
> Chained sg lists will only be feed to a scsi host controller that
> enables chaining in its host template.
> 
> The fix looks fine though, it's just not a requirement or bug fix :-)

Good, then the sbp2 and fw-sbp2 patches can wait for 2.6.25.

Which criteria decide whether a SCSI low-level driver should enable
chained s/g lists?  The SBP-2 protocol supports s/g lists with up to
65535 entries.  The sbp2 and fw-sbp2 driver limit this currently to SG_ALL.
-- 
Stefan Richter
-=====-=-=== =-== ---=-
http://arcgraph.de/sr/

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Jens Axboe]

On Sun, Nov 04 2007, Stefan Richter wrote:
> Jens Axboe wrote:
> > Chained sg lists will only be feed to a scsi host controller that
> > enables chaining in its host template.
> > 
> > The fix looks fine though, it's just not a requirement or bug fix :-)
> 
> Good, then the sbp2 and fw-sbp2 patches can wait for 2.6.25.
> 
> Which criteria decide whether a SCSI low-level driver should enable
> chained s/g lists?  The SBP-2 protocol supports s/g lists with up to
> 65535 entries.  The sbp2 and fw-sbp2 driver limit this currently to SG_ALL.

If the driver can benefit (or even requires) from more segments than you
can typically allocate in one piece, then it should enable chaining. In
general, all drivers must be transitioned to using the sg accessor
helpers, so we can eventually kill the sg chaining enable parameter in
the host template and just make it the default. The parameter is only
there as a transition help.

-- 
Jens Axboe

Re: 2.6.24-rc1-54866f032307063776b4eff7eadb131d47f9f9b4 fails to boot: kernel BUG at include/linux/scatterlist.h:49!

[Torsten Kaiser]

[removing ieee1394 related cc's]

On 11/4/07, Jens Axboe <jens.axboe@oracle.com> wrote:
> Chained sg lists will only be feed to a scsi host controller that
> enables chaining in its host template.
>
> The fix looks fine though, it's just not a requirement or bug fix :-)

I just searched backwards to where the list came from
(scsi_alloc_sgtable()) and did not see any limit there. Also it's
caller did not limit it, but took the value from
req->nr_phys_segments, but then I got lazy and did not check how this
is generated by block/ll_rw_blk.c...

> > As yesterday my md1_raid5-thread oopsed with the same bug from the
> > thread "kernel NULL pointer dereference in blk_rq_map_sg with
> > v2.6.23-6815-g0895e91" I'm rather suspicious of anything sg related
> > right now. (At least I think its the same bug, as 2.6.23-mm1 does not
> > contain the fix from that thread)
>
> Can you post that oops please?

No problem.
I was just doing dd if=/dev/zero of=/home/image bs=1M count=45k and
the the oops took to root filesystem down.

[28241.180000] Unable to handle kernel paging request at ffff810120000000 RIP:

[28241.180000]  [<ffffffff8039ca00>] blk_rq_map_sg+0x70/0x180

[28241.180000] PGD 8063 PUD d063 PMD 0

[28241.180000] Oops: 0000 [1] SMP

[28241.210000] last sysfs file: /block/sdd/stat

[28241.210000] CPU 3

[28241.210000] Modules linked in: nls_iso8859_1 vfat fat ext3 jbd ext2
mbcache radeon drm nfsd exportfs ipv6 w83792d tuner tea5767 tda8290
tuner_simple mt20xx tvaudio msp3400 bttv ir_common compat_ioctl32
videobuf_dma_sg videobuf_core btcx_risc tveeprom videodev usbhid
v4l2_common v4l1_compat hid pata_amd sg i2c_nforce2

[28241.210000] Pid: 946, comm: md1_raid5 Not tainted 2.6.23-mm1 #8

[28241.210000] RIP: 0010:[<ffffffff8039ca00>]  [<ffffffff8039ca00>]
blk_rq_map_sg+0x70/0x180

[28241.210000] RSP: 0018:ffff81000613fc90  EFLAGS: 00010006

[28241.210000] RAX: 000000010151b000 RBX: ffff81011fffffc0 RCX: 00000001018eb000

[28241.210000] RDX: 0000000000000000 RSI: ffff8101014c88d0 RDI: ffff8101014c8868

[28241.210000] RBP: 0000000000002000 R08: ffff81011fffffe0 R09: 0000000000001000

[28241.210000] R10: 0000000000000000 R11: 00000001018ec000 R12: ffff810005e04000

[28241.210000] R13: 0000000000000001 R14: 000000000000007f R15: 00001e0000000000

[28241.210000] FS:  00007f6e752d96f0(0000) GS:ffff810100314700(0000)
knlGS:0000000000000000

[28241.210000] CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b

[28241.210000] CR2: ffff810120000000 CR3: 00000000061b5000 CR4: 00000000000006e0

[28241.210000] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000

[28241.210000] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400

[28241.210000] Process md1_raid5 (pid: 946, threadinfo
ffff81000613e000, task ffff8100060c7530)

[28241.210000] last branch before last exception/interrupt

[28241.210000]  from  [<ffffffff8039cab6>] blk_rq_map_sg+0x126/0x180

[28241.210000]  to  [<ffffffff8039ca00>] blk_rq_map_sg+0x70/0x180

[28241.210000] Stack:  0000000100000000 ffff810105616e00
ffff810101187800 ffff810102e6d7e0

[28241.210000]  0000000000000400 0000000002a46b89 ffff810005e04000
ffffffff804385b5

[28241.210000]  ffff810102e6d7e0 ffff810101187800 ffff810005d3c600
ffffffff80440b98

[28241.210000] Call Trace:

[28241.210000]  [<ffffffff804385b5>] scsi_init_io+0x75/0x100

[28241.210000]  [<ffffffff80440b98>] sd_prep_fn+0x98/0x400

[28241.210000]  [<ffffffff8039b7e5>] elv_next_request+0xf5/0x1f0

[28241.210000]  [<ffffffff8022c8ea>] __wake_up_common+0x5a/0x90

[28241.210000]  [<ffffffff80439229>] scsi_request_fn+0x69/0x360

[28241.210000]  [<ffffffff803a06b8>] generic_unplug_device+0x18/0x30

[28241.210000]  [<ffffffff804b6feb>] unplug_slaves+0x6b/0xc0

[28241.210000]  [<ffffffff804cabd0>] md_thread+0x0/0x100

[28241.210000]  [<ffffffff804bf7bd>] raid5d+0x44d/0x490

[28241.210000]  [<ffffffff805b01d7>] schedule_timeout+0x67/0xd0

[28241.210000]  [<ffffffff805b01ca>] schedule_timeout+0x5a/0xd0

[28241.210000]  [<ffffffff804cabd0>] md_thread+0x0/0x100

[28241.210000]  [<ffffffff804cac00>] md_thread+0x30/0x100

[28241.210000]  [<ffffffff8024a710>] autoremove_wake_function+0x0/0x30

[28241.210000]  [<ffffffff804cabd0>] md_thread+0x0/0x100

[28241.210000]  [<ffffffff8024a32b>] kthread+0x4b/0x80

[28241.210000]  [<ffffffff8020c9d8>] child_rip+0xa/0x12

[28241.210000]  [<ffffffff8024a2e0>] kthread+0x0/0x80

[28241.210000]  [<ffffffff8020c9ce>] child_rip+0x0/0x12

[28241.210000]

[28241.210000]

[28241.210000] Code: 49 8b 40 20 49 8d 48 20 4c 89 c3 48 89 c2 48 83
e2 fe a8 01

[28241.210000] RIP  [<ffffffff8039ca00>] blk_rq_map_sg+0x70/0x180

[28241.210000]  RSP <ffff81000613fc90>

[28241.210000] CR2: ffff810120000000


gdb says:
(gdb) list *0xffffffff8039ca00
0xffffffff8039ca00 is in blk_rq_map_sg (include/linux/scatterlist.h:48).
43       */
44      static inline struct scatterlist *sg_next(struct scatterlist *sg)
45      {
46              sg++;
47
48              if (unlikely(sg_is_chain(sg)))
49                      sg = sg_chain_ptr(sg);
50
51              return sg;
52      }

Torsten

[PATCH] ieee1394: sbp2: fix bogus s/g access change

[Stefan Richter]

sg_dma_len(sg) is invalid before the s/g list is DMA-mapped.

This fixes a post 2.6.24 regression which prevents access to SBP-2
devices on several architectures, introduced by "ieee1394: sbp2: s/g
list access cosmetics", commit 825f1df545ab0289185373b0eaf06fb0b3487422.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/sbp2.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: linux-2.6.24/drivers/ieee1394/sbp2.c
===================================================================
--- linux-2.6.24.orig/drivers/ieee1394/sbp2.c
+++ linux-2.6.24/drivers/ieee1394/sbp2.c
@@ -1465,10 +1465,9 @@ static void sbp2_prep_command_orb_sg(str
 	orb->misc |= ORB_SET_DIRECTION(orb_direction);
 
 	/* special case if only one element (and less than 64KB in size) */
-	if ((scsi_use_sg == 1) &&
-	    (sg_dma_len(sg) <= SBP2_MAX_SG_ELEMENT_LENGTH)) {
+	if (scsi_use_sg == 1 && sg->length <= SBP2_MAX_SG_ELEMENT_LENGTH) {
 
-		cmd->dma_size = sg_dma_len(sg);
+		cmd->dma_size = sg->length;
 		cmd->dma_type = CMD_DMA_PAGE;
 		cmd->cmd_dma = dma_map_page(hi->host->device.parent,
 					    sg_page(sg), sg->offset,

-- 
Stefan Richter
-=====-==--- --=- ----=
http://arcgraph.de/sr/