Re: [PATCH v2 1/1] hugepages: Fix race between hugetlbfs umount and quota update.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/19/2011 04:51 PM, Andrew Morton wrote:
> What's different about hugetlbfs?  Why don't other filesystems hit this?
> 
> <investigates further>
> 
> OK so the incorrect interaction happened in free_huge_page(), which is
> called via the compound page destructor (this dtor is "what's different
> about hugetlbfs").   What is incorrect about this is
> 
> a) that we're doing fs operations in response to a
>    get_user_pages()/put_page() operation which has *nothing* to do with
>    filesystems!
> 
> b) that we continue to try to do that fs operation against an fs
>    which was unmounted and freed three days ago. duh.

Yes.

> So I hereby pronounce that
> 
> a) It was wrong to manipulate hugetlbfs quotas within
>    free_huge_page().  Because free_huge_page() is a low-level
>    page-management function which shouldn't know about one of its
>    specific clients (in this case, hugetlbfs).
> 
>    In fact it's wrong for there to be *any* mention of hugetlbfs
>    within hugetlb.c.
> 
> b) I shouldn't have merged that hugetlbfs quota code.  whodidthat. 
>    Mel, Adam, Dave, at least...
> 
> c) The proper fix here is to get that hugetlbfs quota code out of
>    free_huge_page() and do it all where it belongs: within hugetlbfs
>    code.
> 
> 
> Regular filesystems don't need to diddle quota counts within
> page_cache_release().  Why should hugetlbfs need to?

Is there anyone, more expert in hugetlbfs code than I, who can/should/will take
that on?


>> +#define HPAGE_INACTIVE  0
>> +#define HPAGE_ACTIVE    1
> 
> The above need documenting, please.  That documentation would perhaps
> help me understand why we need both an "active" flag *and* a refcount.

It doesn't need both. Now that you mention it, it would be simpler to put it all
in the refcount. I'd send an updated patch, but it sounds like things will be
going in a different direction.


--
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>


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]