lkml.org 
[lkml]   [2007]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] SPARC64: fix iommu sg chaining
On Wed, Oct 17 2007, David Miller wrote:
> From: Jens Axboe <jens.axboe@oracle.com>
> Date: Wed, 17 Oct 2007 11:16:29 +0200
>
> > 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);
>
> Ok, next bug, introduced by this change:
>
> commit f565913ef8a8d0cfa46a1faaf8340cc357a46f3a
> Author: Jens Axboe <jens.axboe@oracle.com>
> Date: Fri Sep 21 10:44:19 2007 +0200
>
> block: convert to using sg helpers
>
> Convert the main rq mapper (blk_rq_map_sg()) to the sg helper setup.
>
> Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
>
> Specifically this part:
>
> new_segment:
> - memset(&sg[nsegs],0,sizeof(struct scatterlist));
> - sg[nsegs].page = bvec->bv_page;
> - sg[nsegs].length = nbytes;
> - sg[nsegs].offset = bvec->bv_offset;
> + sg = next_sg;
> + next_sg = sg_next(sg);
>
> + sg->page = bvec->bv_page;
> + sg->length = nbytes;
> + sg->offset = bvec->bv_offset;
>
> You can't remove that memset(), it's there for a reason. The IOMMU
> layers depended upon the code zero'ing out the whole scatterlist
> struct, there might be more to it than page, length and offset :-)

I realize that, and I was pretty worried about this specific change. But
there's only been one piece of fallout because if it until now - well
two, with the sparc64 stuff.

The problem is that you cannot zero the entire sg entry, because then
you'd potentially overwrite the chain pointer.

I'd propose just adding a

sg_dma_address(sg) = 0;
sg_dma_len(sg) = 0;

there for now, or provide an arch_clear_sg_entry() helper if we need
more killed.

--
Jens Axboe

-
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 13:01    [W:0.078 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site