Messages in this thread | | | Date | Tue, 26 Mar 2013 08:40:23 -0700 | From | Dave <> | Subject | Re: [PATCHv2, RFC 13/30] thp, mm: implement grab_cache_huge_page_write_begin() |
| |
On 03/26/2013 03:48 AM, Kirill A. Shutemov wrote: > Dave Hansen wrote: >>> +repeat: >>> + page = find_lock_page(mapping, index); >>> + if (page) { >>> + if (!PageTransHuge(page)) { >>> + unlock_page(page); >>> + page_cache_release(page); >>> + return NULL; >>> + } >>> + goto found; >>> + } >>> + >>> + page = alloc_pages(gfp_mask & ~gfp_notmask, HPAGE_PMD_ORDER); >> >> I alluded to this a second ago, but what's wrong with alloc_hugepage()? > > It's defined only for !CONFIG_NUMA and only inside mm/huge_memory.c.
It's a short function, but you could easily pull it out from under the #ifdef and export it. I kinda like the idea of these things being allocated in as few code paths possible. But, it's not a big deal.
>>> +found: >>> + wait_on_page_writeback(page); >>> + return page; >>> +} >>> +#endif >> >> So, I diffed : >> >> -struct page *grab_cache_page_write_begin(struct address_space >> vs. >> +struct page *grab_cache_huge_page_write_begin(struct address_space >> >> They're just to similar to ignore. Please consolidate them somehow. > > Will do. > >>> +found: >>> + wait_on_page_writeback(page); >>> + return page; >>> +} >>> +#endif >> >> In grab_cache_page_write_begin(), this "wait" is: >> >> wait_for_stable_page(page); >> >> Why is it different here? > > It was wait_on_page_writeback() in grab_cache_page_write_begin() when I forked > it :( > > See 1d1d1a7 mm: only enforce stable page writes if the backing device requires it > > Consolidation will fix this.
Excellent.
| |