Messages in this thread | | | From | "Chen, Kenneth W" <> | Subject | RE: [patch] hugetlb strict commit accounting | Date | Thu, 9 Mar 2006 03:43:12 -0800 |
| |
David Gibson wrote on Thursday, March 09, 2006 3:27 AM > Um... as far as I can tell, this patch doesn't actually reserve > anything. There are no changes to the fault handler ot > alloc_huge_page(), so there's nothing to stop PRIVATE mappings dipping > into the supposedly reserved pool.
Well, the reservation is already done at mmap time for shared mapping. Why does kernel need to do anything at fault time? Doing it at fault time is an indication of weakness (or brokenness) - you already promised at mmap time that there will be a page available for faulting. Why check them again at fault time?
I don't think your implementation handles PRIVATE mapping either, Isn't it? Private mapping doesn't enter into the page cache hanging of address_space pointer, so either way, it is busted.
> This looks a bit like a case of "let's make it an atomic_t to sprinkle > it with magic atomicity dust" without thinking about what operations > are and need to be atomic. I think resv_huge_pages should be an > ordinary int, but protected by a lock (exactly which lock is not > immediately obvious).
Yeah, I agree. It crossed my mind whether I should fix that or post a fairly straightforward back port. I decided to do the latter and I got bitten :-( That is in the pipeline if people agree that this variable reservation system is a better one.
> What is the list of regions (mapping->private_list) protected by? > mmap_sem (the only thing I can think of off hand that's already taken) > doesn't cut it, because the mapping can be accessed by multiple mms.
I think it is the inode->i_mutex.
- Ken
- 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/
| |