strict accounting and wasteful reservations
Subsystems (2)
Sorry to bring this up after the strict accounting patch was merged but things moved along a bit too fast for me to intervene.
In the thread beginning at http://lkml.org/lkml/2006/3/8/47 , a discussion was had to compare the patch from David Gibson (the patch that was ultimately merged) with an alternative patch from Ken Chen. The main functional difference is how we handle arbitrary file offsets into a hugetlb file. The current patch reserves enough huge pages to populate the whole file up to the highest file offset in use. Ken's patch supported arbitrary blocks.
For libhugetlbfs, we would like to have sparsely populated hugetlb files without wasting all the extra huge pages that the current implementation requires. That aside, having yet another difference in behavior for hugetlbfs files (that isn't necessary) seems like a bad idea.
So on to my questions. Do people agree that supporting reservation for sparsely populated hugetlbfs files makes sense?
I've been hearing complaints about the code churn in hugetlbfs code lately, so is there a way to adapt what we currently have to support this?
Otherwise, should I (or Ken?) take a stab at resurrecting Ken's competing patch with the intent of eventually replacing the current code?
Signature · 8 lines
Adam Litke - (agl at us.ibm.com) IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 11 lines · Adam Litke
Sorry to bring this up after the strict accounting patch was merged but things moved along a bit too fast for me to intervene.
In the thread beginning at http://lkml.org/lkml/2006/3/8/47 , a discussion was had to compare the patch from David Gibson (the patch that was ultimately merged) with an alternative patch from Ken Chen. The main functional difference is how we handle arbitrary file offsets into a hugetlb file. The current patch reserves enough huge pages to populate the whole file up to the highest file offset in use. Ken's patch supported arbitrary blocks.
Quoted 4 lines
For libhugetlbfs, we would like to have sparsely populated hugetlb files without wasting all the extra huge pages that the current implementation requires. That aside, having yet another difference in behavior for hugetlbfs files (that isn't necessary) seems like a bad idea.
We would? Why?
Quoted 10 lines
So on to my questions. Do people agree that supporting reservation for sparsely populated hugetlbfs files makes sense?
I've been hearing complaints about the code churn in hugetlbfs code lately, so is there a way to adapt what we currently have to support this?
Otherwise, should I (or Ken?) take a stab at resurrecting Ken's competing patch with the intent of eventually replacing the current code?
Signature · 10 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 12 lines · David Gibson
Quoted 11 lines · Adam Litke
Sorry to bring this up after the strict accounting patch was merged but things moved along a bit too fast for me to intervene.
In the thread beginning at http://lkml.org/lkml/2006/3/8/47 , a discussion was had to compare the patch from David Gibson (the patch that was ultimately merged) with an alternative patch from Ken Chen. The main functional difference is how we handle arbitrary file offsets into a hugetlb file. The current patch reserves enough huge pages to populate the whole file up to the highest file offset in use. Ken's patch supported arbitrary blocks.
Quoted 6 lines
Quoted 4 lines
For libhugetlbfs, we would like to have sparsely populated hugetlb files without wasting all the extra huge pages that the current implementation requires. That aside, having yet another difference in behavior for hugetlbfs files (that isn't necessary) seems like a bad idea.
We would? Why?
We are thinking about switching the implementation of the ELF segment remapping code to store all of the remapped segments in one hugetlbfs file. That way we have one hugetlb file per executable. This makes managing the segments much easier, especially when doing things like global sharing. When doing this, we'd like the file offset to correspond to the virtual address of the mapped segment. So I admit that altering the kernel behavior helps libhugetlbfs, but I think my second justification above is even more important. I like removing anomalies from hugetlbfs whenever possible.
Signature · 8 lines
Adam Litke - (agl at us.ibm.com) IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 20 lines · Adam Litke
Quoted 12 lines · David Gibson
Quoted 11 lines · Adam Litke
Sorry to bring this up after the strict accounting patch was merged but things moved along a bit too fast for me to intervene.
In the thread beginning at http://lkml.org/lkml/2006/3/8/47 , a discussion was had to compare the patch from David Gibson (the patch that was ultimately merged) with an alternative patch from Ken Chen. The main functional difference is how we handle arbitrary file offsets into a hugetlb file. The current patch reserves enough huge pages to populate the whole file up to the highest file offset in use. Ken's patch supported arbitrary blocks.
Quoted 6 lines
Quoted 4 lines
For libhugetlbfs, we would like to have sparsely populated hugetlb files without wasting all the extra huge pages that the current implementation requires. That aside, having yet another difference in behavior for hugetlbfs files (that isn't necessary) seems like a bad idea.
We would? Why?
Quoted 9 lines
We are thinking about switching the implementation of the ELF segment remapping code to store all of the remapped segments in one hugetlbfs file. That way we have one hugetlb file per executable. This makes managing the segments much easier, especially when doing things like global sharing. When doing this, we'd like the file offset to correspond to the virtual address of the mapped segment. So I admit that altering the kernel behavior helps libhugetlbfs, but I think my second justification above is even more important. I like removing anomalies from hugetlbfs whenever possible.
Hrm... I'm not entirely convinced attempting to directly map vaddr to file offset is a good idea. But give it a shot, I guess.
Signature · 10 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 10 lines · David Gibson
Quoted 9 lines
We are thinking about switching the implementation of the ELF segment remapping code to store all of the remapped segments in one hugetlbfs file. That way we have one hugetlb file per executable. This makes managing the segments much easier, especially when doing things like global sharing. When doing this, we'd like the file offset to correspond to the virtual address of the mapped segment. So I admit that altering the kernel behavior helps libhugetlbfs, but I think my second justification above is even more important. I like removing anomalies from hugetlbfs whenever possible.
Quoted 2 lines
Hrm... I'm not entirely convinced attempting to directly map vaddr to file offset is a good idea. But give it a shot, I guess.
It works, but just wastes a ton of huge pages in the process.
Signature · 8 lines
Adam Litke - (agl at us.ibm.com) IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 11 lines · Adam Litke
Quoted 10 lines · David Gibson
Quoted 9 lines
We are thinking about switching the implementation of the ELF segment remapping code to store all of the remapped segments in one hugetlbfs file. That way we have one hugetlb file per executable. This makes managing the segments much easier, especially when doing things like global sharing. When doing this, we'd like the file offset to correspond to the virtual address of the mapped segment. So I admit that altering the kernel behavior helps libhugetlbfs, but I think my second justification above is even more important. I like removing anomalies from hugetlbfs whenever possible.
Quoted 4 lines
Quoted 2 lines
Hrm... I'm not entirely convinced attempting to directly map vaddr to file offset is a good idea. But give it a shot, I guess.
It works, but just wastes a ton of huge pages in the process.
Just putting the various segments at consecutive offsets from the beginning of the file wouldn't be that hard. It might be a quicker way forward than altering the kernel behaviour.
Signature · 10 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Gibson wrote on Thursday, April 13, 2006 2:32 PM
Quoted 12 lines · Adam Litke
Quoted 11 lines · David Gibson
Quoted 10 lines
Quoted 9 lines
We are thinking about switching the implementation of the ELF segment remapping code to store all of the remapped segments in one hugetlbfs file. That way we have one hugetlb file per executable. This makes managing the segments much easier, especially when doing things like global sharing. When doing this, we'd like the file offset to correspond to the virtual address of the mapped segment. So I admit that altering the kernel behavior helps libhugetlbfs, but I think my second justification above is even more important. I like removing anomalies from hugetlbfs whenever possible.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Hrm... I'm not entirely convinced attempting to directly map vaddr to file offset is a good idea. But give it a shot, I guess.
It works, but just wastes a ton of huge pages in the process.
Just putting the various segments at consecutive offsets from the beginning of the file wouldn't be that hard. It might be a quicker way forward than altering the kernel behaviour.
Arbitrary offset isn't that bad, here is the patch that I forward port to 2.6.17-rc1. It is just 35 lines more. Another thing I can do is to put the variable region tracking code into a library function, maybe that will help to move it along? I'm with Adam, I don't like to see hugetlbfs have yet another uncommon behavior.
- Ken
fs/hugetlbfs/inode.c | 21 +-- include/linux/hugetlb.h | 8 - mm/hugetlb.c | 282 +++++++++++++++++++++++++++--------------------- 3 files changed, 173 insertions(+), 138 deletions(-)
fs/hugetlbfs/inode.c +9 -12
@@ -59,7 +59,6 @@ static void huge_pagevec_release(structstatic int hugetlbfs_file_mmap (struct file *file,struct vm_area_struct *vma) {struct inode *inode = file->f_dentry->d_inode; -struct hugetlbfs_inode_info *info =HUGETLBFS_I (inode);loff_t len, vma_len;int ret;
@@ -87,9 +86,10 @@ static int hugetlbfs_file_mmap(struct fiif (!(vma->vm_flags &VM_WRITE ) && len > inode->i_size) goto out; -if (vma->vm_flags &VM_MAYSHARE ) -if (hugetlb_extend_reservation (info, len >>HPAGE_SHIFT ) !=0 ) - goto out; +if (vma->vm_flags &VM_MAYSHARE && +hugetlb_reserve_pages (inode, vma->vm_pgoff >> (HPAGE_SHIFT -PAGE_SHIFT ), + len >>HPAGE_SHIFT )) + goto out; ret =0 ;hugetlb_prefault_arch_hook (vma->vm_mm);
@@ -195,12 +195,8 @@ static void truncate_hugepages(struct inconst pgoff_t start = lstart >>HPAGE_SHIFT ;struct pagevec pvec;pgoff_t next; -int i; +int i, freed =0 ; -hugetlb_truncate_reservation (HUGETLBFS_I (inode), - lstart >>HPAGE_SHIFT ); -if (!mapping->nrpages) -return ;pagevec_init (&pvec,0 ); next = start;while (1 ) {
@@ -221,10 +217,12 @@ static void truncate_hugepages(struct intruncate_huge_page (page);unlock_page (page);hugetlb_put_quota (mapping); + freed++; }huge_pagevec_release (&pvec); }BUG_ON (!lstart && mapping->nrpages); +hugetlb_unreserve_pages (inode, start, freed); }static void hugetlbfs_delete_inode (struct inode *inode)
@@ -366,6 +364,7 @@ static struct inode *hugetlbfs_get_inode inode->i_mapping->a_ops = &hugetlbfs_aops; inode->i_mapping->backing_dev_info =&hugetlbfs_backing_dev_info; inode->i_atime = inode->i_mtime = inode->i_ctime =CURRENT_TIME ; +INIT_LIST_HEAD (&inode->i_mapping->private_list); info =HUGETLBFS_I (inode);mpol_shared_policy_init (&info->policy,MPOL_DEFAULT ,NULL );switch (mode &S_IFMT ) {
@@ -538,7 +537,6 @@ static struct inode *hugetlbfs_alloc_inohugetlbfs_inc_free_inodes (sbinfo);return NULL ; } - p->prereserved_hpages =0 ;return &p->vfs_inode; }
@@ -781,8 +779,7 @@ struct file *hugetlb_zero_setup(size_t s goto out_file; error = -ENOMEM ; -if (hugetlb_extend_reservation (HUGETLBFS_I (inode), - size >>HPAGE_SHIFT ) !=0 ) +if (hugetlb_reserve_pages (inode,0 , size >>HPAGE_SHIFT )) goto out_inode;d_instantiate (dentry, inode);
include/linux/hugetlb.h +2 -6
@@ -23,6 +23,8 @@ int hugetlb_report_node_meminfo(int, chaunsigned long hugetlb_total_pages (void );int hugetlb_fault (struct mm_struct *mm,struct vm_area_struct *vma,unsigned long address,int write_access); +int hugetlb_reserve_pages (struct inode *inode,long from,long to); +void hugetlb_unreserve_pages (struct inode *inode,long offset,long freed);extern unsigned long max_huge_pages;extern const unsigned long hugetlb_zero, hugetlb_infinity;
@@ -139,8 +141,6 @@ struct hugetlbfs_sb_info {struct hugetlbfs_inode_info {struct shared_policy policy; -/* Protected by the (global) hugetlb_lock */ -unsigned long prereserved_hpages;struct inode vfs_inode; };
@@ -157,10 +157,6 @@ static inline struct hugetlbfs_sb_info *extern const struct file_operations hugetlbfs_file_operations;extern struct vm_operations_struct hugetlb_vm_ops;struct file *hugetlb_zero_setup (size_t ); -int hugetlb_extend_reservation (struct hugetlbfs_inode_info *info, -unsigned long atleast_hpages); -void hugetlb_truncate_reservation (struct hugetlbfs_inode_info *info, -unsigned long atmost_hpages);int hugetlb_get_quota (struct address_space *mapping);void hugetlb_put_quota (struct address_space *mapping);
mm/hugetlb.c +162 -120
325 lines not on this page · raw message
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 5 lines · Kenneth W
Arbitrary offset isn't that bad, here is the patch that I forward port to 2.6.17-rc1. It is just 35 lines more. Another thing I can do is to put the variable region tracking code into a library function, maybe that will help to move it along? I'm with Adam, I don't like to see hugetlbfs have yet another uncommon behavior.
Thanks Ken. The patch passes the libhugetlbfs test suite and also works as advertised for sparse mappings. I don't recall, is this the version you and David were converging on before Dave's patch was merged? I seem to remember a few iterations of this patch centered locking discussions, etc.
Signature · 8 lines
Adam Litke - (agl at us.ibm.com) IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Adam Litke wrote on Friday, April 14, 2006 10:33 AM
Quoted 7 lines · Kenneth W
Quoted 5 lines
Arbitrary offset isn't that bad, here is the patch that I forward port to 2.6.17-rc1. It is just 35 lines more. Another thing I can do is to put the variable region tracking code into a library function, maybe that will help to move it along? I'm with Adam, I don't like to see hugetlbfs have yet another uncommon behavior.
Quoted 5 lines
Thanks Ken. The patch passes the libhugetlbfs test suite and also works as advertised for sparse mappings. I don't recall, is this the version you and David were converging on before Dave's patch was merged? I seem to remember a few iterations of this patch centered locking discussions, etc.
Adam, yes, this is the latest rev includes all the comments from David.
- Ken
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>