[email protected] wrote:
> Convert ieee1394 from nopage to fault.
> Remove redundant vma range checks (correct resource range check is retained).
I never really looked into that part of the 1394 drivers and I'm too
lazy to figure this out myself, so I ask: What would trip the .fault
handler? Would be good if I could runtime-test it.
> Signed-off-by: Nick Piggin <[email protected]>
> Cc: [email protected]
It's an obscure and unimportant detail but I mention it nevertheless:
It's not necessary to Cc krh on drivers/ieee1394 stuff. He is more into
drivers/firewire. :-)
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/ieee1394/dma.c | 39 +++++++++++++++++----------------------
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
> Index: linux-2.6/drivers/ieee1394/dma.c
> ===================================================================
> --- linux-2.6.orig/drivers/ieee1394/dma.c
> +++ linux-2.6/drivers/ieee1394/dma.c
> @@ -231,37 +231,32 @@ void dma_region_sync_for_device(struct d
>
> #ifdef CONFIG_MMU
>
> -/* nopage() handler for mmap access */
> +/* fault() handler for mmap access */
>
> -static struct page *dma_region_pagefault(struct vm_area_struct *area,
> - unsigned long address, int *type)
> +static int dma_region_pagefault(struct vm_area_struct *vma,
> + struct vm_fault *vmf)
> {
> - unsigned long offset;
> unsigned long kernel_virt_addr;
> - struct page *ret = NOPAGE_SIGBUS;
>
> - struct dma_region *dma = (struct dma_region *)area->vm_private_data;
> + struct dma_region *dma = (struct dma_region *)vma->vm_private_data;
>
> if (!dma->kvirt)
> - goto out;
> + goto error;
>
> - if ((address < (unsigned long)area->vm_start) ||
> - (address >
> - (unsigned long)area->vm_start + (dma->n_pages << PAGE_SHIFT)))
> - goto out;
> -
> - if (type)
> - *type = VM_FAULT_MINOR;
> - offset = address - area->vm_start;
> - kernel_virt_addr = (unsigned long)dma->kvirt + offset;
> - ret = vmalloc_to_page((void *)kernel_virt_addr);
> - get_page(ret);
> - out:
> - return ret;
> + if (vmf->pgoff >= dma->n_pages)
> + goto error;
> +
> + kernel_virt_addr = (unsigned long)dma->kvirt + (vmf->pgoff << PAGE_SHIFT);
> + vmf->page = vmalloc_to_page((void *)kernel_virt_addr);
> + get_page(vmf->page);
> + return 0;
> +
> + error:
> + return VM_FAULT_SIGBUS;
> }
Why not replacing the two 'goto error' by 'return VM_FAULT_SIGBUS'? If
there is no cleanup after that error jump, I find it sensible to return
early.
>
> static struct vm_operations_struct dma_region_vm_ops = {
> - .nopage = dma_region_pagefault,
> + .fault = dma_region_pagefault,
> };
>
> /**
> @@ -275,7 +270,7 @@ int dma_region_mmap(struct dma_region *d
> if (!dma->kvirt)
> return -EINVAL;
>
> - /* must be page-aligned */
> + /* must be page-aligned (XXX: comment is wrong, we could allow pgoff) */
> if (vma->vm_pgoff != 0)
> return -EINVAL;
>
Are you sure that the comment is wrong? Could it be that there are
assumptions elsewhere which require page alignment? (I should be able
to answer that, but I'm not.)
--
Stefan Richter
-=====-=-=== ==-- --=-=
http://arcgraph.de/sr/
--
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]
