lkml.org 
[lkml]   [2011]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/1] hugepages: Fix race between hugetlbfs umount and quota update.
On Wed, 17 Aug 2011 17:01:15 -0500
Andrew Barry <abarry@cray.com> 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.


\
 
 \ /
  Last update: 2011-08-19 00:31    [W:0.029 / U:0.532 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site