Re: [PATCH 1/3] mm: add comment for __mod_zone_page_stat

[Jianyu Zhan]

On Mon, May 12, 2014 at 10:01 PM, Christoph Lameter <cl@xxxxxxxxx> wrote:
> >
> >/*
> > * For use when we know that interrupts are disabled,
> > * or when we know that preemption is disabled and that
> > * particular counter cannot be updated from interrupt context.
> > */
>
> The description above looks ok to me. The problem is that you are
> considering the page related data structures as an issue for races and not
> the data structures relevant for vm statistics.

Hi,  Christoph,

Yep. I did.

Let me restate the point here.

 To use  __mod_zone_page_stat when we know that either
 1. interrupts are disabled, or
 2. preemption is disabled and that particular counter cannot be
    updated from interrupt context.

For those call sites currently using __mod_zone_page_stat, they just guarantees
the counter is never modified from an interrupt context, but doesn't disable
preemption.

This means they guarantee that even they are preemted the vm
counter won't be modified incorrectly.  Because the counter is page-related
(e.g., a new anon page added), and they are exclusively hold the pte lock.

This is why I emphasized on 'the page related data structures as an
issue for races'.

So, as you concludes in the other mail that __modd_zone_page_stat
couldn't be used.
in mlocked_vma_newpage, then what qualifies other call sites for using
it, in the same situation?
See:

void page_add_new_anon_rmap(struct page *page,
        struct vm_area_struct *vma, unsigned long address)
{
        VM_BUG_ON(address < vma->vm_start || address >= vma->vm_end);
        SetPageSwapBacked(page);
        atomic_set(&page->_mapcount, 0); /* increment count (starts at -1) */
        if (PageTransHuge(page))
                __inc_zone_page_state(page, NR_ANON_TRANSPARENT_HUGEPAGES);
        __mod_zone_page_state(page_zone(page), NR_ANON_PAGES,
                        hpage_nr_pages(page));         <---  using it.
        __page_set_anon_rmap(page, vma, address, 1);
        if (!mlocked_vma_newpage(vma, page)) {     <--- couldn't use it ?
                SetPageActive(page);
                lru_cache_add(page);
        } else
                add_page_to_unevictable_list(page);
}

Hope I express it clearly enough.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/