Re: Implement mmap for PUD

[David Young <dyoung%pobox.com@localhost>]

On Wed, Sep 07, 2011 at 10:33:54AM +0200, Roger Pau Monné wrote:
> Hello,
> 
> Since there is no mmap implementation for PUD devices I began working
> on one. I would like to make an implementation that avoids copying
> buffers around user-space and kernel memory, since mmap is usually
> used for fast applications. I began working on pud_dev.c file, that
> contains the kernel implementation of the mmap call, which is then
> passed to user space. My mmap function looks like:
> 
> 
> static paddr_t
> pud_cdev_mmap(dev_t dev, off_t off, int flag)
> {
>       struct pud_creq_mmap pc_mmap;
>       struct vmspace *vm;
>       int error;
>       int num;
>       paddr_t pa;
> 
>       pc_mmap.pm_flag  = flag;
>       pc_mmap.pm_pageoff = off;
> 
>       printf("Inside mmap, off: %jd flag: %d\n", (intmax_t) off, flag);
> 
>       error = pud_request(dev, &pc_mmap, sizeof(pc_mmap),
>           PUD_REQ_CDEV, PUD_CDEV_MMAP);
>       if (error)
>               return (paddr_t) -1;
> 
>       mutex_enter(proc_lock);
>       pc_mmap.pm_proc = proc_find(pc_mmap.pid);
>       mutex_exit(proc_lock);
>       /* Catch error? */
> 
>       error = proc_vmspace_getref(pc_mmap.pm_proc, &vm);
>       if (error)
>               panic("Unable to get vmspace");
> 
>       /* Try to read the value */
>       if(copyin_proc(pc_mmap.pm_proc, (void *) pc_mmap.pm_addr, &num,
> sizeof(num)) == EFAULT)
>               panic("Unable to read value from user-space");
>       printf("Read: %d from addr: %" PRIxPADDR "\n", num, pc_mmap.pm_addr);
> 
>       if ((vaddr_t)pc_mmap.pm_addr & PAGE_MASK)
>               panic("pud_cdev_mmap: memory not page aligned");
> 
>       if (pmap_extract(vm->vm_map.pmap,
>           (vaddr_t) pc_mmap.pm_addr + (u_int) off, &pa) == FALSE)
>               panic("pud_cdev_mmap: memory page not mapped");
> 
>       uvmspace_free(vm);
>       printf("Inside mmap, returning: %" PRIxPADDR "\n", pa);
> 
>       return pa;
> }

I think that you need to return atop(pa) instead of pa.  I.e., a
driver's mmap() returns a physical page number, not a physical address.
Is this unnecessarily confusing?  Yes. :-)  Documented in any manual
page?  Maybe not.

I'm curious whether the pa -> atop(pa) change helps.

> void * memory;
> 
> vaddr_t
> test_mmap(dev_t dev, off_t off, int flag, void *auxdata)
> {
>       int *num;
>       if (off > 0)
>               return (vaddr_t)-1;
>       if (memory == NULL)
>       {
>               memory = malloc(PAGE_SIZE);
>               if (mlock(memory, PAGE_SIZE) < 0)
>                       err(EXIT_FAILURE, "Unable to lock pages");

It's probably not desirable to mlock() every PUD device page, but if the
PUD device page gets paged out, what do the userspace device and/or the
kernel need to do about paging it back in?

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 344-0444 x24