lkml.org 
[lkml]   [2007]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [bug] ata subsystem related crash with latest -git
    From
    Date
    On Wed, 17 Oct 2007 14:11:34 -0700 (PDT)
    Linus Torvalds <torvalds@linux-foundation.org> wrote:

    >
    >
    > On Wed, 17 Oct 2007, Jens Axboe wrote:
    > >
    > > That would hurt... Care to commit your for_each_sg() uglification fixup
    > > for now then? Or disable the allocation debug config entry, so that the
    > > sg+1 deref wont crash?
    >
    > Well, in practice, it will only crash with DEBUG_PAGEALLOC, so few enough
    > are going to be hit by it. In that sense I don't think we're in any deep
    > trouble yet.
    >
    > That said, maybe this is an acceptable, if hacky, replacement for the
    > current "for_each_sg()" loop.
    >
    > It does:
    > - starts at one *before* the sglist
    > - does the sg_next() at the *top* of the loop rather than the bottom of it
    > - has a "--count" before that sg_next, so that we don't do it for the
    > case when we break out and have used up all segments.
    >
    > Totally untested, but it *may* work, and it doesn't look horribly ugly.
    >
    > Ingo, does this actually make any difference?
    >
    > Linus
    >
    > ---
    > include/linux/scatterlist.h | 9 ++++++++-
    > 1 files changed, 8 insertions(+), 1 deletions(-)
    >
    > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
    > index 2dc7464..f5c8e11 100644
    > --- a/include/linux/scatterlist.h
    > +++ b/include/linux/scatterlist.h
    > @@ -51,11 +51,18 @@ static inline struct scatterlist *sg_next(struct scatterlist *sg)
    > return sg;
    > }
    >
    > +static inline struct scatterlist *sg_safe_next(struct scatterlist *sg, int left)
    > +{
    > + if (left < 0)
    > + return NULL;
    > + return sg_next(sg);
    > +}
    > +
    > /*
    > * Loop over each sg element, following the pointer to a new list if necessary
    > */
    > #define for_each_sg(sglist, sg, nr, __i) \
    > - for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
    > + for (__i = (nr), sg = (sglist)-1; (sg = sg_safe_next(sg, --__i)) != NULL ; )

    Looks that (sglist) - 1 isn't initialized and we use sg_next for it?
    -
    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-18 02:25    [W:4.400 / U:0.144 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site