Messages in this thread Patch in this message | | | Date | Fri, 11 Nov 2005 16:25:11 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] 2.6.14 patch for supporting madvise(MADV_REMOVE) |
| |
Badari Pulavarty <pbadari@us.ibm.com> wrote: > > +/* > + * Application wants to free up the pages and associated backing store. > + * This is effectively punching a hole into the middle of a file. > + * > + * NOTE: Currently, only shmfs/tmpfs is supported for this operation. > + * Other filesystems return -ENOSYS. > + */ > +static long madvise_remove(struct vm_area_struct * vma, > + unsigned long start, unsigned long end) > +{ > + struct address_space *mapping; > + loff_t offset, endoff; > + > + if (vma->vm_flags & (VM_LOCKED|VM_NONLINEAR|VM_HUGETLB)) > + return -EINVAL; > + > + if (!vma->vm_file || !vma->vm_file->f_mapping > + || !vma->vm_file->f_mapping->host) { > + return -EINVAL; > + } > + > + mapping = vma->vm_file->f_mapping; > + if (mapping == &swapper_space) { > + return -EINVAL; > + } > + > + offset = (loff_t)(start - vma->vm_start) > + + (vma->vm_pgoff << PAGE_SHIFT); > + endoff = (loff_t)(end - vma->vm_start - 1) > + + (vma->vm_pgoff << PAGE_SHIFT); > + return vmtruncate_range(mapping->host, offset, endoff); > +} > +
I'm suspecting you tested this on a 64-bit machine, yes? On 32-bit that vm_pgoff shift is going to overflow.
Fixes-thus-far below. Please rerun all tests on x86?
Why does madvise_remove() have an explicit check for swapper_space?
In your testing, how are you determining that the code is successfully removing the correct number of pages, from the correct file offset?
diff -puN mm/madvise.c~madvise-remove-remove-pages-from-tmpfs-shm-backing-store-tidy mm/madvise.c --- devel/mm/madvise.c~madvise-remove-remove-pages-from-tmpfs-shm-backing-store-tidy 2005-11-11 16:12:43.000000000 -0800 +++ devel-akpm/mm/madvise.c 2005-11-11 16:16:50.000000000 -0800 @@ -147,8 +147,8 @@ static long madvise_dontneed(struct vm_a * NOTE: Currently, only shmfs/tmpfs is supported for this operation. * Other filesystems return -ENOSYS. */ -static long madvise_remove(struct vm_area_struct * vma, - unsigned long start, unsigned long end) +static long madvise_remove(struct vm_area_struct *vma, + unsigned long start, unsigned long end) { struct address_space *mapping; loff_t offset, endoff; @@ -162,14 +162,13 @@ static long madvise_remove(struct vm_are } mapping = vma->vm_file->f_mapping; - if (mapping == &swapper_space) { + if (mapping == &swapper_space) return -EINVAL; - } offset = (loff_t)(start - vma->vm_start) - + (vma->vm_pgoff << PAGE_SHIFT); + + ((loff_t)vma->vm_pgoff << PAGE_SHIFT); endoff = (loff_t)(end - vma->vm_start - 1) - + (vma->vm_pgoff << PAGE_SHIFT); + + ((loff_t)vma->vm_pgoff << PAGE_SHIFT); return vmtruncate_range(mapping->host, offset, endoff); } diff -puN mm/memory.c~madvise-remove-remove-pages-from-tmpfs-shm-backing-store-tidy mm/memory.c --- devel/mm/memory.c~madvise-remove-remove-pages-from-tmpfs-shm-backing-store-tidy 2005-11-11 16:16:54.000000000 -0800 +++ devel-akpm/mm/memory.c 2005-11-11 16:17:59.000000000 -0800 @@ -1608,10 +1608,9 @@ out_big: out_busy: return -ETXTBSY; } - EXPORT_SYMBOL(vmtruncate); -int vmtruncate_range(struct inode * inode, loff_t offset, loff_t end) +int vmtruncate_range(struct inode *inode, loff_t offset, loff_t end) { struct address_space *mapping = inode->i_mapping; @@ -1634,7 +1633,6 @@ int vmtruncate_range(struct inode * inod return 0; } - EXPORT_SYMBOL(vmtruncate_range); /* _ - 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/
| |