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    [W:0.025 / U:116.816 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site