Re: User mode drivers: part 2: PCI device handling (patch 1/2 for 2.6.11)

[Andrew Grover <andy.grover@gmail.com>]

On Fri, 11 Mar 2005 14:37:17 +1100, Peter Chubb
<peterc@gelato.unsw.edu.au> wrote:

> +       npages = get_user_pages(current,
> +                               current->mm,
> +                               (unsigned long)m.virtaddr,
> +                               maxpages,
> +                               write,
> +                               0,
> +                               imp->pages,
> +                               NULL);

Can't comment on usermode drivers overall, so just some code comments.

do you need a down_read(current->mm->mmap_sem) here? I'm not sure but
that seems to be what most other users of this function are doing.

> +       /*
> +        * Build scatterlist, one entry per page.
> +        * Allow for partial pages at start and end.
> +        */
> +       i = 1;
> +       imp->sg[0].page = imp->pages[0];
> +       imp->sg[0].offset = ((unsigned long)m.virtaddr) & (PAGE_SIZE - 1);
> +       imp->sg[0].length = PAGE_SIZE - imp->sg[0].offset;
> +       if (imp->sg[0].length >= m.size) {
> +               imp->sg[0].length = m.size;
> +       } else {
> +               unsigned long len = m.size - imp->sg[0].length;
> +               for (;len >= PAGE_SIZE && i < npages ; i++) {
> +                       imp->sg[i].page = imp->pages[i];
> +                       imp->sg[i].offset = 0;
> +                       imp->sg[i].length = PAGE_SIZE;
> +                       len -= PAGE_SIZE;
> +               }
> +               if (len) {
> +                       BUG_ON(i >= npages);
> +                       BUG_ON(len >= PAGE_SIZE);
> +                       imp->sg[i].page = imp->pages[i];
> +                       imp->sg[i].offset = 0;
> +                       imp->sg[i].length = len;
> +                       i++;
> +               }
> +       }

size_t len = m.size;
int offset = (unsigned long) m.virtaddr & ~PAGE_MASK;
for (i = 0; i < npages; i++)
{
  imp->sg[i].page = imp->pages[i];
  imp->sg[i].offset = offset;
  imp->sg[i].length = min(len, PAGE_SIZE - offset);

  offset = 0;
  len -= imp->sg[i].length;
}

instead possibly?

Regards -- Andy