lkml.org 
[lkml]   [2013]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] mm: introduce __linear_page_index()
On Tue, 5 Feb 2013 15:03:39 +0800
Huang Shijie <b32955@freescale.com> wrote:

> There are many places we should get the offset(in PAGE_SIZE unit) of
> an address within a non-hugetlb vma.
>
> In order to simplify the code, add a new helper __linear_page_index()
> to do the work.
>

Seems nice.

> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -310,15 +310,23 @@ static inline loff_t page_file_offset(struct page *page)
> extern pgoff_t linear_hugepage_index(struct vm_area_struct *vma,
> unsigned long address);
>
> -static inline pgoff_t linear_page_index(struct vm_area_struct *vma,
> +/* The offset for an address within a non-hugetlb vma, in PAGE_SIZE unit. */

"The offset into the mapped file for ..."

> +static inline pgoff_t __linear_page_index(struct vm_area_struct *vma,
> unsigned long address)
> {
> pgoff_t pgoff;
> +
> + pgoff = (address - vma->vm_start) >> PAGE_SHIFT;
> + return pgoff + vma->vm_pgoff;
> +}
> +
> +static inline pgoff_t linear_page_index(struct vm_area_struct *vma,
> + unsigned long address)
> +{
> if (unlikely(is_vm_hugetlb_page(vma)))
> return linear_hugepage_index(vma, address);
> - pgoff = (address - vma->vm_start) >> PAGE_SHIFT;
> - pgoff += vma->vm_pgoff;
> - return pgoff >> (PAGE_CACHE_SHIFT - PAGE_SHIFT);
> + return __linear_page_index(vma, address) >>
> + (PAGE_CACHE_SHIFT - PAGE_SHIFT);
> }

I don't think we need bother creating both linear_page_index() and
__linear_page_index(). Realistically, we won't be supporting
PAGE_SHIFT!=PAGE_CACHE_SHIFT. And most (or all?) of the sites which
you changed should have been using PAGE_CACHE_SHIFT anyway!

> @@ -1201,8 +1199,7 @@ SYSCALL_DEFINE1(shmdt, char __user *, shmaddr)
>
> /* finding a matching vma now does not alter retval */
> if ((vma->vm_ops == &shm_vm_ops) &&
> - (vma->vm_start - addr)/PAGE_SIZE == vma->vm_pgoff)
> -
> + 0 == __linear_page_index(vma, addr))

erk, please don't do this - it makes kernel developers fall over in
shock. Let's do

__linear_page_index(vma, addr) == 0

(This won't compile if someone forgets a `=', so the usual reason for
the backward comparison isn't valid).




\
 
 \ /
  Last update: 2013-02-05 23:02    [W:0.041 / U:8.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site