lkml.org 
[lkml]   [2015]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv3 04/24] rmap: add argument to charge compound page
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?



\
 
 \ /
  Last update: 2015-03-04 13:21    [W:0.108 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site