Messages in this thread | | | Date | Wed, 28 Sep 2005 23:20:27 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 3/3 htlb-acct] Demand faulting for huge pages |
| |
Adam Litke <agl@us.ibm.com> wrote: > > Initial Post (Thu, 18 Aug 2005) > > Basic overcommit checking for hugetlb_file_map() based on an implementation > used with demand faulting in SLES9. > > Since demand faulting can't guarantee the availability of pages at mmap time, > this patch implements a basic sanity check to ensure that the number of huge > pages required to satisfy the mmap are currently available. Despite the > obvious race, I think it is a good start on doing proper accounting. I'd like > to work towards an accounting system that mimics the semantics of normal pages > (especially for the MAP_PRIVATE/COW case). That work is underway and builds on > what this patch starts. > > Huge page shared memory segments are simpler and still maintain their commit on > shmget semantics. > > Diffed against 2.6.14-rc2-git6 > > Signed-off-by: Adam Litke <agl@us.ibm.com> > --- > inode.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 47 insertions(+) > diff -upN reference/fs/hugetlbfs/inode.c current/fs/hugetlbfs/inode.c > --- reference/fs/hugetlbfs/inode.c > +++ current/fs/hugetlbfs/inode.c > @@ -45,9 +45,51 @@ static struct backing_dev_info hugetlbfs > > int sysctl_hugetlb_shm_group; > > +static void huge_pagevec_release(struct pagevec *pvec);
nit: personally I prefer to move the helper function to the top of the file rather than having to forward-declare it.
> +unsigned long > +huge_pages_needed(struct address_space *mapping, struct vm_area_struct *vma) > +{
What does this function do? Seems to count all the present pages within a vma which are backed by a particular hugetlbfs file? Or something?
If so, the chosen name seems strange. And it definitely needs a decent comment.
> + int i; > + struct pagevec pvec; > + unsigned long start = vma->vm_start; > + unsigned long end = vma->vm_end; > + unsigned long hugepages = (end - start) >> HPAGE_SHIFT;
`hugepages' is the size of the vma
> + pgoff_t next = vma->vm_pgoff; > + pgoff_t endpg = next + ((end - start) >> PAGE_SHIFT); > + struct inode *inode = vma->vm_file->f_dentry->d_inode; > + > + /* > + * Shared memory segments are accounted for at shget time, > + * not at shmat (when the mapping is actually created) so > + * check here if the memory has already been accounted for. > + */ > + if (inode->i_blocks != 0) > + return 0; > + > + pagevec_init(&pvec, 0); > + while (next < endpg) { > + if (!pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE)) > + break; > + for (i = 0; i < pagevec_count(&pvec); i++) { > + struct page *page = pvec.pages[i]; > + if (page->index > next) > + next = page->index; > + if (page->index >= endpg) > + break; > + next++; > + hugepages--;
And we subtract one from it for each present page.
> + } > + huge_pagevec_release(&pvec); > + } > + return hugepages << HPAGE_SHIFT; > +}
So it seems to be returning the number of bytes which are still unpopulated within this vma?
Think you can rework this code to reduce my perplexity?
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |