lkml.org 
[lkml]   [2007]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] SPARC64: fix iommu sg chaining
From
Date
On Wed, 17 Oct 2007 11:16:29 +0200
Jens Axboe <jens.axboe@oracle.com> wrote:

> On Wed, Oct 17 2007, David Miller wrote:
> > From: Jens Axboe <jens.axboe@oracle.com>
> > Date: Wed, 17 Oct 2007 10:45:28 +0200
> >
> > > Righto, it's invalid to call sg_next() on the last entry!
> >
> > Unfortunately, that's what the sparc64 code wanted to do, this
> > transformation in the sparc64 sg chaining patch is not equilavent:
> >
> > - struct scatterlist *sg_end = sg + nelems;
> > + struct scatterlist *sg_end = sg_last(sg, nelems);
> > ...
> > - while (sg < sg_end &&
> > + while (sg != sg_end &&
>
> Auch indeed. That'd probably be better as a
>
> do {
> ...
> } while (sg != sg_end);
>
> > No, that's not what the code was doing. The while loop
> > has to process the last entry in the list,
> >
> > We really needed "sg_end" to be "one past the last element",
> > rather than "the last element".
> >
> > Since you say that sg_next() on the last entry is illegal,
> > and that's what this code would have done to try and reach
> > loop termination (it doesn't actually derefrence that
> > "end plus one" scatterlist entry) I'll try to code this up
> > some other way.
> >
> > Besides, sg_last() is so absurdly expensive, it has to walk the entire
> > chain in the chaining case. So better to implement this without it.
>
> It is, sg_last() should really not be used a lot since it'll leaf
> through the entire sg list. People should either keep count of the
> number of entries so that they know when they are dealing with the last
> valid entry. Or use the for_each_sg() loop helper, if possible.
>
> Drivers are usually very simple, the iommu code does more sg tricks and
> thus is more complex to audit.

Can we just remove sg_last?


> > I would suggest that other sg_last() uses be audited for the same bug.
>
> Agree.

Only libata, I think.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-10-17 11:27    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site