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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
