On Wed, 17 Aug 2011 17:01:15 -0500 Andrew Barry <abarry@xxxxxxxx> wrote: > This patch fixes a race between the umount of a hugetlbfs filesystem, and quota > updates in that filesystem, which can result in the update of the filesystem > quota record, after the record structure has been freed. > > Rather than an address-space struct pointer, it puts a hugetlbfs_sb_info struct > pointer into page_private of the page struct. A reference count and an active > bit are added to the hugetlbfs_sb_info struct; the reference count is increased > by hugetlb_get_quota and decreased by hugetlb_put_quota. When hugetlbfs is > unmounted, it frees the hugetlbfs_sb_info struct, but only if the reference > count is zero, otherwise it clears the active bit. The last hugetlb_put_quota > then frees the hugetlbfs_sb_info struct. > > Discussion was titled: Fix refcounting in hugetlbfs quota handling. > See: https://lkml.org/lkml/2011/8/11/28 The changelog doesn't actually describe the race - it just asserts that there is one. This makes it unnecessarily difficult to review the fix! So I didn't really look at the code - I just scanned the trivial stuff. The patch was somewhat wordwrapped - please fix the email client then resend. > + if (hugetlb_get_quota(HUGETLBFS_SB(inode->i_mapping->host->i_sb), chg)) > + hugetlb_put_quota(HUGETLBFS_SB(inode->i_mapping->host->i_sb), chg); > + set_page_private(page, (unsigned long)HUGETLBFS_SB(inode->i_mapping->host->i_sb)); > + hugetlb_put_quota(HUGETLBFS_SB(vma->vm_file->f_mapping->host->i_sb), reserve); > + if (hugetlb_get_quota(HUGETLBFS_SB(inode->i_mapping->host->i_sb), chg)) > + hugetlb_put_quota(HUGETLBFS_SB(inode->i_mapping->host->i_sb), chg); > + hugetlb_put_quota(HUGETLBFS_SB(inode->i_mapping->host->i_sb), (chg - freed)); Are all the inode->i_mapping->host pointer hops actually necessary? I didn't see anything about them in the changelog and I'd expect that inode->i_mapping->host is always equal to `inode' for hugetlbfs? If they _are_ necessary then I'd suggest that the code could be cleaned up by adding static struct hugetlbfs_sb_info *inode_to_sb(struct inode *inode) { return HUGETLBFS_SB(inode->i_mapping->host->i_sb); } to hugetlbfs.c. This will reduce the relatively large number of checkpatch warnings which were added. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>