lkml.org 
[lkml]   [2007]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 08/10] [SG] Update arch/ to use sg helpers
On Mon, Oct 22 2007, Benny Halevy wrote:
> It looks like it could be nice to define and use a helper for
> page_address(sg_page(sg)) (although 11 call sites could use it
> after this patch)
>
> #define sg_pgaddr(sg) page_address(sg_page(sg))
>
> Note that mips sg_{un,}map_sg checked for page_address(sg->page) != 0
> before calling __dma_sync(addr + sg->offset, sg->length, direction)
> and you changed it to addr = (unsigned long) sg_virt(sg) which
> takes sg->offset into account. That said I'm not sure if the original
> code was correct for the (page_address(sg->page) == 0 && sg->offset != 0)
> case...

I initially thought that may have been a bug, but most of the other arch
iommu code handle it in the same way. I don't think it's a bug, since
they want to clear full page ranges anyway. I should not have changed
this code to take the offset into account - I don't think it's an issue,
but it should have been left as-is. Will do that.

> > diff --git a/arch/x86/kernel/pci-calgary_64.c b/arch/x86/kernel/pci-calgary_64.c
> > index 5098f58..1a20fe3 100644
> > --- a/arch/x86/kernel/pci-calgary_64.c
> > +++ b/arch/x86/kernel/pci-calgary_64.c
> > @@ -411,8 +411,10 @@ static int calgary_nontranslate_map_sg(struct device* dev,
> > int i;
> >
> > for_each_sg(sg, s, nelems, i) {
> > - BUG_ON(!s->page);
> > - s->dma_address = virt_to_bus(page_address(s->page) +s->offset);
> > + struct page *p = sg_page(s);
> > +
> > + BUG_ON(!p);
>
> why not just BUG_ON(!sg_page(s))?
>
> > + s->dma_address = virt_to_bus(sg_virt(s));
> > s->dma_length = s->length;
> > }
> > return nelems;

I think because of a two stage conversion, the page variable addition
predates the sg_virt() helper.

--
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-23 09:31    [W:0.110 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site