Re: [PATCHv3 04/24] rmap: add argument to charge compound page

Vlastimil Babka
2015-03-04T12:09:57Z

On 03/04/2015 12:52 PM, Kirill A. Shutemov wrote:
> On Mon, Feb 23, 2015 at 05:21:31PM +0100, Vlastimil Babka wrote:
>> On 02/12/2015 05:18 PM, Kirill A. Shutemov wrote:
>>> @@ -1052,21 +1052,24 @@ void page_add_anon_rmap(struct page *page,
>>> * Everybody else should continue to use page_add_anon_rmap above.
>>> */
>>> void do_page_add_anon_rmap(struct page *page,
>>> - struct vm_area_struct *vma, unsigned long address, int exclusive)
>>> + struct vm_area_struct *vma, unsigned long address, int flags)
>>> {
>>> int first = atomic_inc_and_test(&page->_mapcount);
>>> if (first) {
>>> + bool compound = flags & RMAP_COMPOUND;
>>> + int nr = compound ? hpage_nr_pages(page) : 1;
>>
>> hpage_nr_pages(page) is:
>>
>> static inline int hpage_nr_pages(struct page *page)
>> {
>> if (unlikely(PageTransHuge(page)))
>> return HPAGE_PMD_NR;
>> return 1;
>> }
>>
>> and later...
>>
>>> /*
>>> * We use the irq-unsafe __{inc|mod}_zone_page_stat because
>>> * these counters are not modified in interrupt context, and
>>> * pte lock(a spinlock) is held, which implies preemption
>>> * disabled.
>>> */
>>> - if (PageTransHuge(page))
>>> + if (compound) {
>>> + VM_BUG_ON_PAGE(!PageTransHuge(page), page);
>>
>> this means that we could assume that
>> (compound == true) => (PageTransHuge(page) == true)
>>
>> and simplify above to:
>>
>> int nr = compound ? HPAGE_PMD_NR : 1;
>>
>> Right?
>
> No. HPAGE_PMD_NR is defined based on HPAGE_PMD_SHIFT which is BUILD_BUG()
> without CONFIG_TRANSPARENT_HUGEPAGE. We will get compiler error without
> the helper.

Oh, OK. But that doesn't mean there couldn't be another helper that
would work in this case, or even open-coded #ifdefs in these functions.
Apparently "compound" has to be always false for
!CONFIG_TRANSPARENT_HUGEPAGE, as in that case PageTransHuge is defined
as 0 and the VM_BUG_ON would trigger if compound was true. So without
such ifdefs or wrappers, you are also adding dead code and pointless
tests for !CONFIG_TRANSPARENT_HUGEPAGE?