Re: sata_sil24 broken since 2.6.23-rc4-mm1

["Torsten Kaiser" <just.for.lkml@googlemail.com>]

[Adding Jens Axboe, the author of what looks like the probable cause]
On 10/7/07, Torsten Kaiser <just.for.lkml@googlemail.com> wrote:
> My sil24_fill_sg now looks like this:
> static inline void sil24_fill_sg(struct ata_queued_cmd *qc,
>                                  struct sil24_sge *sge)
> {
>         struct scatterlist *sg;
>
>         ata_for_each_sg(sg, qc) {
>                 sge->addr = cpu_to_le64(sg_dma_address(sg));
>                 sge->cnt = cpu_to_le32(sg_dma_len(sg));
>                 if (ata_sg_is_last(sg, qc))
>                         sge->flags = cpu_to_le32(SGE_TRM);
>                 else
>                         sge->flags = 0;
>                 DPRINTK("flags,addr,cnt = 0x%x, 0x%X, 0x%X\n", sge->flags,
>                         sge->addr, sge->cnt);
>                 sge++;
>         }
> }
>
> Suspicious is, that *all* output from this DPRINTK shows flags as 0x0,
> so the last sg is never terminated (SGE_TRM is 1<<31)?
> But if that is the cause, how is this working at all? Or am I doing
> something stupid?

Looking closer at
http://git.kernel.org/?p=linux/kernel/git/axboe/linux-2.6-block.git;a=commitdiff;h=ec6fdded4d76aa54aa57341e5dfdd61c507b1dcd
the change to libata.h seems bogus :

in ata_qc_first_sg:
old                                new
return qc->__sg                    return qc->__sg
qc->__sg - qc->__sg == 0           qc->n_iter=0
-> sg - qc->__sg corresponds to qc->n_iter

in ata_qc_next_sg:
sg++;                              sg_next(sg); qc->n_iter++;
sg - qc->__sg < qc->n_elem         qc->n_iter < qc->nelem
-> sg - qc->__sg corresponds to qc->n_iter

but in ata_sg_is_last:
(sg - qc->__sg) +1 == qc->n_elem   qc->n_iter == qc->n_elem
if sg - qc->__sg corresponds to qc->n_iter then shoudn't it be
qc->n_iter+1 == qc->n_elem?

That missing +1 would explain, why the SGE_TRM never gets set.

And it would fit the symptoms, that the boot would fail at random. If
the "correct" garbage was in place to where the sglist runs off it
hangs the drive.
And that would even fit the two different errors that I only got one time each:
* a completely illegal access (PCI master abort while fetching SGT)
* wrong alignment of the SGT (SGT no on qword boundary)
At that that times the garbage seemed to point invalid addresses.

But I'm still not understanding, how the kernel could only fail
sometimes at bootup, but after that working without any visible
errors? Is the sil-chip rather intelligent about detecting corrupted
sglists and silently ignoring them?

Torsten