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

2015-03-04 Thread Kirill A. Shutemov
On Wed, Mar 04, 2015 at 01:09:51PM +0100, Vlastimil Babka wrote:
> 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(>_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?

Yeah, this definitely can be improved. I prefer to do it as follow up.
I want to get stable what I have now.

-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2015-03-04 Thread Vlastimil Babka

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(>_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?


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


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

2015-03-04 Thread Kirill A. Shutemov
On Mon, Feb 23, 2015 at 05:21:31PM +0100, Vlastimil Babka wrote:
> On 02/12/2015 05:18 PM, Kirill A. Shutemov wrote:
> > We're going to allow mapping of individual 4k pages of THP compound
> > page. It means we cannot rely on PageTransHuge() check to decide if map
> > small page or THP.
> > 
> > The patch adds new argument to rmap function to indicate whethe we want
> > to map whole compound page or only the small page.
> > 
> > Signed-off-by: Kirill A. Shutemov 
> > ---
> >  include/linux/rmap.h| 14 +++---
> >  kernel/events/uprobes.c |  4 ++--
> >  mm/huge_memory.c| 16 
> >  mm/hugetlb.c|  4 ++--
> >  mm/ksm.c|  4 ++--
> >  mm/memory.c | 14 +++---
> >  mm/migrate.c|  8 
> >  mm/rmap.c   | 43 +++
> >  mm/swapfile.c   |  4 ++--
> >  9 files changed, 65 insertions(+), 46 deletions(-)
> > 
> > diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> > index c4088feac1fc..3bf73620b672 100644
> > --- a/include/linux/rmap.h
> > +++ b/include/linux/rmap.h
> > @@ -168,16 +168,24 @@ static inline void anon_vma_merge(struct 
> > vm_area_struct *vma,
> >  
> >  struct anon_vma *page_get_anon_vma(struct page *page);
> >  
> > +/* flags for do_page_add_anon_rmap() */
> > +enum {
> > +   RMAP_EXCLUSIVE = 1,
> > +   RMAP_COMPOUND = 2,
> > +};
> > +
> >  /*
> >   * rmap interfaces called when adding or removing pte of page
> >   */
> >  void page_move_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
> > long);
> > -void page_add_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
> > long);
> > +void page_add_anon_rmap(struct page *, struct vm_area_struct *,
> > +   unsigned long, bool);
> >  void do_page_add_anon_rmap(struct page *, struct vm_area_struct *,
> >unsigned long, int);
> > -void page_add_new_anon_rmap(struct page *, struct vm_area_struct *, 
> > unsigned long);
> > +void page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
> > +   unsigned long, bool);
> >  void page_add_file_rmap(struct page *);
> > -void page_remove_rmap(struct page *);
> > +void page_remove_rmap(struct page *, bool);
> >  
> >  void hugepage_add_anon_rmap(struct page *, struct vm_area_struct *,
> > unsigned long);
> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> > index cb346f26a22d..5523daf59953 100644
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -183,7 +183,7 @@ static int __replace_page(struct vm_area_struct *vma, 
> > unsigned long addr,
> > goto unlock;
> >  
> > get_page(kpage);
> > -   page_add_new_anon_rmap(kpage, vma, addr);
> > +   page_add_new_anon_rmap(kpage, vma, addr, false);
> > mem_cgroup_commit_charge(kpage, memcg, false);
> > lru_cache_add_active_or_unevictable(kpage, vma);
> >  
> > @@ -196,7 +196,7 @@ static int __replace_page(struct vm_area_struct *vma, 
> > unsigned long addr,
> > ptep_clear_flush_notify(vma, addr, ptep);
> > set_pte_at_notify(mm, addr, ptep, mk_pte(kpage, vma->vm_page_prot));
> >  
> > -   page_remove_rmap(page);
> > +   page_remove_rmap(page, false);
> > if (!page_mapped(page))
> > try_to_free_swap(page);
> > pte_unmap_unlock(ptep, ptl);
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 5f4c97e1a6da..36637a80669e 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -743,7 +743,7 @@ static int __do_huge_pmd_anonymous_page(struct 
> > mm_struct *mm,
> > pmd_t entry;
> > entry = mk_huge_pmd(page, vma->vm_page_prot);
> > entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
> > -   page_add_new_anon_rmap(page, vma, haddr);
> > +   page_add_new_anon_rmap(page, vma, haddr, true);
> > mem_cgroup_commit_charge(page, memcg, false);
> > lru_cache_add_active_or_unevictable(page, vma);
> > pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > @@ -1034,7 +1034,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
> > mm_struct *mm,
> > entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> > memcg = (void *)page_private(pages[i]);
> > set_page_private(pages[i], 0);
> > -   page_add_new_anon_rmap(pages[i], vma, haddr);
> > +   page_add_new_anon_rmap(pages[i], vma, haddr, false);
> > mem_cgroup_commit_charge(pages[i], memcg, false);
> > lru_cache_add_active_or_unevictable(pages[i], vma);
> > pte = pte_offset_map(&_pmd, haddr);
> > @@ -1046,7 +1046,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
> > mm_struct *mm,
> >  
> > smp_wmb(); /* make pte visible before pmd */
> > pmd_populate(mm, pmd, pgtable);
> > -   page_remove_rmap(page);
> > +   page_remove_rmap(page, true);
> > spin_unlock(ptl);
> >  
> > mmu_notifier_invalidate_range_end(mm, 

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

2015-03-04 Thread Kirill A. Shutemov
On Mon, Feb 23, 2015 at 05:21:31PM +0100, Vlastimil Babka wrote:
 On 02/12/2015 05:18 PM, Kirill A. Shutemov wrote:
  We're going to allow mapping of individual 4k pages of THP compound
  page. It means we cannot rely on PageTransHuge() check to decide if map
  small page or THP.
  
  The patch adds new argument to rmap function to indicate whethe we want
  to map whole compound page or only the small page.
  
  Signed-off-by: Kirill A. Shutemov kirill.shute...@linux.intel.com
  ---
   include/linux/rmap.h| 14 +++---
   kernel/events/uprobes.c |  4 ++--
   mm/huge_memory.c| 16 
   mm/hugetlb.c|  4 ++--
   mm/ksm.c|  4 ++--
   mm/memory.c | 14 +++---
   mm/migrate.c|  8 
   mm/rmap.c   | 43 +++
   mm/swapfile.c   |  4 ++--
   9 files changed, 65 insertions(+), 46 deletions(-)
  
  diff --git a/include/linux/rmap.h b/include/linux/rmap.h
  index c4088feac1fc..3bf73620b672 100644
  --- a/include/linux/rmap.h
  +++ b/include/linux/rmap.h
  @@ -168,16 +168,24 @@ static inline void anon_vma_merge(struct 
  vm_area_struct *vma,
   
   struct anon_vma *page_get_anon_vma(struct page *page);
   
  +/* flags for do_page_add_anon_rmap() */
  +enum {
  +   RMAP_EXCLUSIVE = 1,
  +   RMAP_COMPOUND = 2,
  +};
  +
   /*
* rmap interfaces called when adding or removing pte of page
*/
   void page_move_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
  long);
  -void page_add_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
  long);
  +void page_add_anon_rmap(struct page *, struct vm_area_struct *,
  +   unsigned long, bool);
   void do_page_add_anon_rmap(struct page *, struct vm_area_struct *,
 unsigned long, int);
  -void page_add_new_anon_rmap(struct page *, struct vm_area_struct *, 
  unsigned long);
  +void page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
  +   unsigned long, bool);
   void page_add_file_rmap(struct page *);
  -void page_remove_rmap(struct page *);
  +void page_remove_rmap(struct page *, bool);
   
   void hugepage_add_anon_rmap(struct page *, struct vm_area_struct *,
  unsigned long);
  diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
  index cb346f26a22d..5523daf59953 100644
  --- a/kernel/events/uprobes.c
  +++ b/kernel/events/uprobes.c
  @@ -183,7 +183,7 @@ static int __replace_page(struct vm_area_struct *vma, 
  unsigned long addr,
  goto unlock;
   
  get_page(kpage);
  -   page_add_new_anon_rmap(kpage, vma, addr);
  +   page_add_new_anon_rmap(kpage, vma, addr, false);
  mem_cgroup_commit_charge(kpage, memcg, false);
  lru_cache_add_active_or_unevictable(kpage, vma);
   
  @@ -196,7 +196,7 @@ static int __replace_page(struct vm_area_struct *vma, 
  unsigned long addr,
  ptep_clear_flush_notify(vma, addr, ptep);
  set_pte_at_notify(mm, addr, ptep, mk_pte(kpage, vma-vm_page_prot));
   
  -   page_remove_rmap(page);
  +   page_remove_rmap(page, false);
  if (!page_mapped(page))
  try_to_free_swap(page);
  pte_unmap_unlock(ptep, ptl);
  diff --git a/mm/huge_memory.c b/mm/huge_memory.c
  index 5f4c97e1a6da..36637a80669e 100644
  --- a/mm/huge_memory.c
  +++ b/mm/huge_memory.c
  @@ -743,7 +743,7 @@ static int __do_huge_pmd_anonymous_page(struct 
  mm_struct *mm,
  pmd_t entry;
  entry = mk_huge_pmd(page, vma-vm_page_prot);
  entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
  -   page_add_new_anon_rmap(page, vma, haddr);
  +   page_add_new_anon_rmap(page, vma, haddr, true);
  mem_cgroup_commit_charge(page, memcg, false);
  lru_cache_add_active_or_unevictable(page, vma);
  pgtable_trans_huge_deposit(mm, pmd, pgtable);
  @@ -1034,7 +1034,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
  mm_struct *mm,
  entry = maybe_mkwrite(pte_mkdirty(entry), vma);
  memcg = (void *)page_private(pages[i]);
  set_page_private(pages[i], 0);
  -   page_add_new_anon_rmap(pages[i], vma, haddr);
  +   page_add_new_anon_rmap(pages[i], vma, haddr, false);
  mem_cgroup_commit_charge(pages[i], memcg, false);
  lru_cache_add_active_or_unevictable(pages[i], vma);
  pte = pte_offset_map(_pmd, haddr);
  @@ -1046,7 +1046,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
  mm_struct *mm,
   
  smp_wmb(); /* make pte visible before pmd */
  pmd_populate(mm, pmd, pgtable);
  -   page_remove_rmap(page);
  +   page_remove_rmap(page, true);
  spin_unlock(ptl);
   
  mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
  @@ -1168,7 +1168,7 @@ alloc:
  entry = mk_huge_pmd(new_page, vma-vm_page_prot);
  entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
  

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

2015-03-04 Thread Vlastimil Babka

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?


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2015-03-04 Thread Kirill A. Shutemov
On Wed, Mar 04, 2015 at 01:09:51PM +0100, Vlastimil Babka wrote:
 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?

Yeah, this definitely can be improved. I prefer to do it as follow up.
I want to get stable what I have now.

-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2015-02-23 Thread Vlastimil Babka
On 02/12/2015 05:18 PM, Kirill A. Shutemov wrote:
> We're going to allow mapping of individual 4k pages of THP compound
> page. It means we cannot rely on PageTransHuge() check to decide if map
> small page or THP.
> 
> The patch adds new argument to rmap function to indicate whethe we want
> to map whole compound page or only the small page.
> 
> Signed-off-by: Kirill A. Shutemov 
> ---
>  include/linux/rmap.h| 14 +++---
>  kernel/events/uprobes.c |  4 ++--
>  mm/huge_memory.c| 16 
>  mm/hugetlb.c|  4 ++--
>  mm/ksm.c|  4 ++--
>  mm/memory.c | 14 +++---
>  mm/migrate.c|  8 
>  mm/rmap.c   | 43 +++
>  mm/swapfile.c   |  4 ++--
>  9 files changed, 65 insertions(+), 46 deletions(-)
> 
> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> index c4088feac1fc..3bf73620b672 100644
> --- a/include/linux/rmap.h
> +++ b/include/linux/rmap.h
> @@ -168,16 +168,24 @@ static inline void anon_vma_merge(struct vm_area_struct 
> *vma,
>  
>  struct anon_vma *page_get_anon_vma(struct page *page);
>  
> +/* flags for do_page_add_anon_rmap() */
> +enum {
> + RMAP_EXCLUSIVE = 1,
> + RMAP_COMPOUND = 2,
> +};
> +
>  /*
>   * rmap interfaces called when adding or removing pte of page
>   */
>  void page_move_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
> long);
> -void page_add_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
> long);
> +void page_add_anon_rmap(struct page *, struct vm_area_struct *,
> + unsigned long, bool);
>  void do_page_add_anon_rmap(struct page *, struct vm_area_struct *,
>  unsigned long, int);
> -void page_add_new_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
> long);
> +void page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
> + unsigned long, bool);
>  void page_add_file_rmap(struct page *);
> -void page_remove_rmap(struct page *);
> +void page_remove_rmap(struct page *, bool);
>  
>  void hugepage_add_anon_rmap(struct page *, struct vm_area_struct *,
>   unsigned long);
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index cb346f26a22d..5523daf59953 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -183,7 +183,7 @@ static int __replace_page(struct vm_area_struct *vma, 
> unsigned long addr,
>   goto unlock;
>  
>   get_page(kpage);
> - page_add_new_anon_rmap(kpage, vma, addr);
> + page_add_new_anon_rmap(kpage, vma, addr, false);
>   mem_cgroup_commit_charge(kpage, memcg, false);
>   lru_cache_add_active_or_unevictable(kpage, vma);
>  
> @@ -196,7 +196,7 @@ static int __replace_page(struct vm_area_struct *vma, 
> unsigned long addr,
>   ptep_clear_flush_notify(vma, addr, ptep);
>   set_pte_at_notify(mm, addr, ptep, mk_pte(kpage, vma->vm_page_prot));
>  
> - page_remove_rmap(page);
> + page_remove_rmap(page, false);
>   if (!page_mapped(page))
>   try_to_free_swap(page);
>   pte_unmap_unlock(ptep, ptl);
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 5f4c97e1a6da..36637a80669e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -743,7 +743,7 @@ static int __do_huge_pmd_anonymous_page(struct mm_struct 
> *mm,
>   pmd_t entry;
>   entry = mk_huge_pmd(page, vma->vm_page_prot);
>   entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
> - page_add_new_anon_rmap(page, vma, haddr);
> + page_add_new_anon_rmap(page, vma, haddr, true);
>   mem_cgroup_commit_charge(page, memcg, false);
>   lru_cache_add_active_or_unevictable(page, vma);
>   pgtable_trans_huge_deposit(mm, pmd, pgtable);
> @@ -1034,7 +1034,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
> mm_struct *mm,
>   entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>   memcg = (void *)page_private(pages[i]);
>   set_page_private(pages[i], 0);
> - page_add_new_anon_rmap(pages[i], vma, haddr);
> + page_add_new_anon_rmap(pages[i], vma, haddr, false);
>   mem_cgroup_commit_charge(pages[i], memcg, false);
>   lru_cache_add_active_or_unevictable(pages[i], vma);
>   pte = pte_offset_map(&_pmd, haddr);
> @@ -1046,7 +1046,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
> mm_struct *mm,
>  
>   smp_wmb(); /* make pte visible before pmd */
>   pmd_populate(mm, pmd, pgtable);
> - page_remove_rmap(page);
> + page_remove_rmap(page, true);
>   spin_unlock(ptl);
>  
>   mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
> @@ -1168,7 +1168,7 @@ alloc:
>   entry = mk_huge_pmd(new_page, vma->vm_page_prot);
>   entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
>   

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

2015-02-23 Thread Vlastimil Babka
On 02/12/2015 05:18 PM, Kirill A. Shutemov wrote:
 We're going to allow mapping of individual 4k pages of THP compound
 page. It means we cannot rely on PageTransHuge() check to decide if map
 small page or THP.
 
 The patch adds new argument to rmap function to indicate whethe we want
 to map whole compound page or only the small page.
 
 Signed-off-by: Kirill A. Shutemov kirill.shute...@linux.intel.com
 ---
  include/linux/rmap.h| 14 +++---
  kernel/events/uprobes.c |  4 ++--
  mm/huge_memory.c| 16 
  mm/hugetlb.c|  4 ++--
  mm/ksm.c|  4 ++--
  mm/memory.c | 14 +++---
  mm/migrate.c|  8 
  mm/rmap.c   | 43 +++
  mm/swapfile.c   |  4 ++--
  9 files changed, 65 insertions(+), 46 deletions(-)
 
 diff --git a/include/linux/rmap.h b/include/linux/rmap.h
 index c4088feac1fc..3bf73620b672 100644
 --- a/include/linux/rmap.h
 +++ b/include/linux/rmap.h
 @@ -168,16 +168,24 @@ static inline void anon_vma_merge(struct vm_area_struct 
 *vma,
  
  struct anon_vma *page_get_anon_vma(struct page *page);
  
 +/* flags for do_page_add_anon_rmap() */
 +enum {
 + RMAP_EXCLUSIVE = 1,
 + RMAP_COMPOUND = 2,
 +};
 +
  /*
   * rmap interfaces called when adding or removing pte of page
   */
  void page_move_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
 long);
 -void page_add_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
 long);
 +void page_add_anon_rmap(struct page *, struct vm_area_struct *,
 + unsigned long, bool);
  void do_page_add_anon_rmap(struct page *, struct vm_area_struct *,
  unsigned long, int);
 -void page_add_new_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
 long);
 +void page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
 + unsigned long, bool);
  void page_add_file_rmap(struct page *);
 -void page_remove_rmap(struct page *);
 +void page_remove_rmap(struct page *, bool);
  
  void hugepage_add_anon_rmap(struct page *, struct vm_area_struct *,
   unsigned long);
 diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
 index cb346f26a22d..5523daf59953 100644
 --- a/kernel/events/uprobes.c
 +++ b/kernel/events/uprobes.c
 @@ -183,7 +183,7 @@ static int __replace_page(struct vm_area_struct *vma, 
 unsigned long addr,
   goto unlock;
  
   get_page(kpage);
 - page_add_new_anon_rmap(kpage, vma, addr);
 + page_add_new_anon_rmap(kpage, vma, addr, false);
   mem_cgroup_commit_charge(kpage, memcg, false);
   lru_cache_add_active_or_unevictable(kpage, vma);
  
 @@ -196,7 +196,7 @@ static int __replace_page(struct vm_area_struct *vma, 
 unsigned long addr,
   ptep_clear_flush_notify(vma, addr, ptep);
   set_pte_at_notify(mm, addr, ptep, mk_pte(kpage, vma-vm_page_prot));
  
 - page_remove_rmap(page);
 + page_remove_rmap(page, false);
   if (!page_mapped(page))
   try_to_free_swap(page);
   pte_unmap_unlock(ptep, ptl);
 diff --git a/mm/huge_memory.c b/mm/huge_memory.c
 index 5f4c97e1a6da..36637a80669e 100644
 --- a/mm/huge_memory.c
 +++ b/mm/huge_memory.c
 @@ -743,7 +743,7 @@ static int __do_huge_pmd_anonymous_page(struct mm_struct 
 *mm,
   pmd_t entry;
   entry = mk_huge_pmd(page, vma-vm_page_prot);
   entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
 - page_add_new_anon_rmap(page, vma, haddr);
 + page_add_new_anon_rmap(page, vma, haddr, true);
   mem_cgroup_commit_charge(page, memcg, false);
   lru_cache_add_active_or_unevictable(page, vma);
   pgtable_trans_huge_deposit(mm, pmd, pgtable);
 @@ -1034,7 +1034,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
 mm_struct *mm,
   entry = maybe_mkwrite(pte_mkdirty(entry), vma);
   memcg = (void *)page_private(pages[i]);
   set_page_private(pages[i], 0);
 - page_add_new_anon_rmap(pages[i], vma, haddr);
 + page_add_new_anon_rmap(pages[i], vma, haddr, false);
   mem_cgroup_commit_charge(pages[i], memcg, false);
   lru_cache_add_active_or_unevictable(pages[i], vma);
   pte = pte_offset_map(_pmd, haddr);
 @@ -1046,7 +1046,7 @@ static int do_huge_pmd_wp_page_fallback(struct 
 mm_struct *mm,
  
   smp_wmb(); /* make pte visible before pmd */
   pmd_populate(mm, pmd, pgtable);
 - page_remove_rmap(page);
 + page_remove_rmap(page, true);
   spin_unlock(ptl);
  
   mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
 @@ -1168,7 +1168,7 @@ alloc:
   entry = mk_huge_pmd(new_page, vma-vm_page_prot);
   entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
   pmdp_clear_flush_notify(vma, haddr, pmd);
 - page_add_new_anon_rmap(new_page, vma, haddr);
 + 

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

2015-02-20 Thread Jerome Marchand
On 02/16/2015 04:20 PM, Kirill A. Shutemov wrote:
> On Thu, Feb 12, 2015 at 04:10:21PM -0500, Rik van Riel wrote:
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> On 02/12/2015 11:18 AM, Kirill A. Shutemov wrote:
>>
>>> +++ b/include/linux/rmap.h @@ -168,16 +168,24 @@ static inline void
>>> anon_vma_merge(struct vm_area_struct *vma,
>>>
>>> struct anon_vma *page_get_anon_vma(struct page *page);
>>>
>>> +/* flags for do_page_add_anon_rmap() */ +enum { +  RMAP_EXCLUSIVE =
>>> 1, +RMAP_COMPOUND = 2, +};
>>
>> Always a good idea to name things. However, "exclusive" is
>> not that clear to me. Given that the argument is supposed
>> to indicate whether we map a single or a compound page,
>> maybe the names in the enum could just be SINGLE and COMPOUND?
>>
>> Naming the enum should make it clear enough what it does:
>>
>>  enum rmap_page {
>>   SINGLE = 0,
>>   COMPOUND
>>  }
> 
> Okay, this is probably confusing: do_page_add_anon_rmap() already had one
> of arguments called 'exclusive'. It indicates if the page is exclusively
> owned by the current process. And I needed also to indicate if we need to
> handle the page as a compound or not. I've reused the same argument and
> converted it to set bit-flags: bit 0 is exclusive, bit 1 - compound.

AFAICT, this is not a common use of enum and probably the reason why Rik
was confused (I know I find it confusing). Bit-flags members are usually
define by macros.

Jerome
> 
>>
>>> +++ b/kernel/events/uprobes.c @@ -183,7 +183,7 @@ static int
>>> __replace_page(struct vm_area_struct *vma, unsigned long addr, goto
>>> unlock;
>>>
>>> get_page(kpage); -  page_add_new_anon_rmap(kpage, vma, addr); +
>>> page_add_new_anon_rmap(kpage, vma, addr, false); 
>>> mem_cgroup_commit_charge(kpage, memcg, false); 
>>> lru_cache_add_active_or_unevictable(kpage, vma);
>>
>> Would it make sense to use the name in the argument to that function,
>> too?
>>
>> I often find it a lot easier to see what things do if they use symbolic
>> names, rather than by trying to remember what each boolean argument to
>> a function does.
> 
> I can convert these compound booleans to enums if you want. I'm personally
> not sure that if will bring much value.
> 




signature.asc
Description: OpenPGP digital signature


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

2015-02-20 Thread Jerome Marchand
On 02/16/2015 04:20 PM, Kirill A. Shutemov wrote:
 On Thu, Feb 12, 2015 at 04:10:21PM -0500, Rik van Riel wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 On 02/12/2015 11:18 AM, Kirill A. Shutemov wrote:

 +++ b/include/linux/rmap.h @@ -168,16 +168,24 @@ static inline void
 anon_vma_merge(struct vm_area_struct *vma,

 struct anon_vma *page_get_anon_vma(struct page *page);

 +/* flags for do_page_add_anon_rmap() */ +enum { +  RMAP_EXCLUSIVE =
 1, +RMAP_COMPOUND = 2, +};

 Always a good idea to name things. However, exclusive is
 not that clear to me. Given that the argument is supposed
 to indicate whether we map a single or a compound page,
 maybe the names in the enum could just be SINGLE and COMPOUND?

 Naming the enum should make it clear enough what it does:

  enum rmap_page {
   SINGLE = 0,
   COMPOUND
  }
 
 Okay, this is probably confusing: do_page_add_anon_rmap() already had one
 of arguments called 'exclusive'. It indicates if the page is exclusively
 owned by the current process. And I needed also to indicate if we need to
 handle the page as a compound or not. I've reused the same argument and
 converted it to set bit-flags: bit 0 is exclusive, bit 1 - compound.

AFAICT, this is not a common use of enum and probably the reason why Rik
was confused (I know I find it confusing). Bit-flags members are usually
define by macros.

Jerome
 

 +++ b/kernel/events/uprobes.c @@ -183,7 +183,7 @@ static int
 __replace_page(struct vm_area_struct *vma, unsigned long addr, goto
 unlock;

 get_page(kpage); -  page_add_new_anon_rmap(kpage, vma, addr); +
 page_add_new_anon_rmap(kpage, vma, addr, false); 
 mem_cgroup_commit_charge(kpage, memcg, false); 
 lru_cache_add_active_or_unevictable(kpage, vma);

 Would it make sense to use the name in the argument to that function,
 too?

 I often find it a lot easier to see what things do if they use symbolic
 names, rather than by trying to remember what each boolean argument to
 a function does.
 
 I can convert these compound booleans to enums if you want. I'm personally
 not sure that if will bring much value.
 




signature.asc
Description: OpenPGP digital signature


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

2015-02-16 Thread Kirill A. Shutemov
On Thu, Feb 12, 2015 at 04:10:21PM -0500, Rik van Riel wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> On 02/12/2015 11:18 AM, Kirill A. Shutemov wrote:
> 
> > +++ b/include/linux/rmap.h @@ -168,16 +168,24 @@ static inline void
> > anon_vma_merge(struct vm_area_struct *vma,
> > 
> > struct anon_vma *page_get_anon_vma(struct page *page);
> > 
> > +/* flags for do_page_add_anon_rmap() */ +enum { +  RMAP_EXCLUSIVE =
> > 1, +RMAP_COMPOUND = 2, +};
> 
> Always a good idea to name things. However, "exclusive" is
> not that clear to me. Given that the argument is supposed
> to indicate whether we map a single or a compound page,
> maybe the names in the enum could just be SINGLE and COMPOUND?
> 
> Naming the enum should make it clear enough what it does:
> 
>  enum rmap_page {
>   SINGLE = 0,
>   COMPOUND
>  }

Okay, this is probably confusing: do_page_add_anon_rmap() already had one
of arguments called 'exclusive'. It indicates if the page is exclusively
owned by the current process. And I needed also to indicate if we need to
handle the page as a compound or not. I've reused the same argument and
converted it to set bit-flags: bit 0 is exclusive, bit 1 - compound.

> 
> > +++ b/kernel/events/uprobes.c @@ -183,7 +183,7 @@ static int
> > __replace_page(struct vm_area_struct *vma, unsigned long addr, goto
> > unlock;
> > 
> > get_page(kpage); -  page_add_new_anon_rmap(kpage, vma, addr); +
> > page_add_new_anon_rmap(kpage, vma, addr, false); 
> > mem_cgroup_commit_charge(kpage, memcg, false); 
> > lru_cache_add_active_or_unevictable(kpage, vma);
> 
> Would it make sense to use the name in the argument to that function,
> too?
> 
> I often find it a lot easier to see what things do if they use symbolic
> names, rather than by trying to remember what each boolean argument to
> a function does.

I can convert these compound booleans to enums if you want. I'm personally
not sure that if will bring much value.

-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2015-02-16 Thread Kirill A. Shutemov
On Thu, Feb 12, 2015 at 04:10:21PM -0500, Rik van Riel wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 On 02/12/2015 11:18 AM, Kirill A. Shutemov wrote:
 
  +++ b/include/linux/rmap.h @@ -168,16 +168,24 @@ static inline void
  anon_vma_merge(struct vm_area_struct *vma,
  
  struct anon_vma *page_get_anon_vma(struct page *page);
  
  +/* flags for do_page_add_anon_rmap() */ +enum { +  RMAP_EXCLUSIVE =
  1, +RMAP_COMPOUND = 2, +};
 
 Always a good idea to name things. However, exclusive is
 not that clear to me. Given that the argument is supposed
 to indicate whether we map a single or a compound page,
 maybe the names in the enum could just be SINGLE and COMPOUND?
 
 Naming the enum should make it clear enough what it does:
 
  enum rmap_page {
   SINGLE = 0,
   COMPOUND
  }

Okay, this is probably confusing: do_page_add_anon_rmap() already had one
of arguments called 'exclusive'. It indicates if the page is exclusively
owned by the current process. And I needed also to indicate if we need to
handle the page as a compound or not. I've reused the same argument and
converted it to set bit-flags: bit 0 is exclusive, bit 1 - compound.

 
  +++ b/kernel/events/uprobes.c @@ -183,7 +183,7 @@ static int
  __replace_page(struct vm_area_struct *vma, unsigned long addr, goto
  unlock;
  
  get_page(kpage); -  page_add_new_anon_rmap(kpage, vma, addr); +
  page_add_new_anon_rmap(kpage, vma, addr, false); 
  mem_cgroup_commit_charge(kpage, memcg, false); 
  lru_cache_add_active_or_unevictable(kpage, vma);
 
 Would it make sense to use the name in the argument to that function,
 too?
 
 I often find it a lot easier to see what things do if they use symbolic
 names, rather than by trying to remember what each boolean argument to
 a function does.

I can convert these compound booleans to enums if you want. I'm personally
not sure that if will bring much value.

-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2015-02-12 Thread Rik van Riel
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/12/2015 11:18 AM, Kirill A. Shutemov wrote:

> +++ b/include/linux/rmap.h @@ -168,16 +168,24 @@ static inline void
> anon_vma_merge(struct vm_area_struct *vma,
> 
> struct anon_vma *page_get_anon_vma(struct page *page);
> 
> +/* flags for do_page_add_anon_rmap() */ +enum { +RMAP_EXCLUSIVE =
> 1, +  RMAP_COMPOUND = 2, +};

Always a good idea to name things. However, "exclusive" is
not that clear to me. Given that the argument is supposed
to indicate whether we map a single or a compound page,
maybe the names in the enum could just be SINGLE and COMPOUND?

Naming the enum should make it clear enough what it does:

 enum rmap_page {
  SINGLE = 0,
  COMPOUND
 }

> +++ b/kernel/events/uprobes.c @@ -183,7 +183,7 @@ static int
> __replace_page(struct vm_area_struct *vma, unsigned long addr, goto
> unlock;
> 
> get_page(kpage); -page_add_new_anon_rmap(kpage, vma, addr); +
> page_add_new_anon_rmap(kpage, vma, addr, false); 
> mem_cgroup_commit_charge(kpage, memcg, false); 
> lru_cache_add_active_or_unevictable(kpage, vma);

Would it make sense to use the name in the argument to that function,
too?

I often find it a lot easier to see what things do if they use symbolic
names, rather than by trying to remember what each boolean argument to
a function does.

- -- 
All rights reversed
-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQEcBAEBAgAGBQJU3Ra9AAoJEM553pKExN6D4UcH/10GlcYBB813KE7dR2r23MDx
WlrcC096IRoEjD/aaBHikLcKSu5mZDzf3ic1ZHzMPzz7oMdsFkmnY/k2zMdcqc83
7scvd7VB3acI4STKWcbkaCsIHIpHPFmfdcLv9Rabi0P2MBb8SALQCwxDUJqvXojC
JdJivfuagDoSUEamHwZrCvFylC7J7M4zPLD5aUpc93E4I4lhG9VHD7FmnYP3rxb8
kX4DOZFZ7aTN3K9IweCZPN2HWZe2qcSKc/AmIfHfokdjJLTuqbMv5UGSwLHmmeDf
DO4Uru/BMgPg2Ds7uKZosf7icAnOzT08b/Woh34JT83ua9XpFMam+hx6g+lA78E=
=Kzss
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2015-02-12 Thread Kirill A. Shutemov
We're going to allow mapping of individual 4k pages of THP compound
page. It means we cannot rely on PageTransHuge() check to decide if map
small page or THP.

The patch adds new argument to rmap function to indicate whethe we want
to map whole compound page or only the small page.

Signed-off-by: Kirill A. Shutemov 
---
 include/linux/rmap.h| 14 +++---
 kernel/events/uprobes.c |  4 ++--
 mm/huge_memory.c| 16 
 mm/hugetlb.c|  4 ++--
 mm/ksm.c|  4 ++--
 mm/memory.c | 14 +++---
 mm/migrate.c|  8 
 mm/rmap.c   | 43 +++
 mm/swapfile.c   |  4 ++--
 9 files changed, 65 insertions(+), 46 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index c4088feac1fc..3bf73620b672 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -168,16 +168,24 @@ static inline void anon_vma_merge(struct vm_area_struct 
*vma,
 
 struct anon_vma *page_get_anon_vma(struct page *page);
 
+/* flags for do_page_add_anon_rmap() */
+enum {
+   RMAP_EXCLUSIVE = 1,
+   RMAP_COMPOUND = 2,
+};
+
 /*
  * rmap interfaces called when adding or removing pte of page
  */
 void page_move_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
long);
-void page_add_anon_rmap(struct page *, struct vm_area_struct *, unsigned long);
+void page_add_anon_rmap(struct page *, struct vm_area_struct *,
+   unsigned long, bool);
 void do_page_add_anon_rmap(struct page *, struct vm_area_struct *,
   unsigned long, int);
-void page_add_new_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
long);
+void page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
+   unsigned long, bool);
 void page_add_file_rmap(struct page *);
-void page_remove_rmap(struct page *);
+void page_remove_rmap(struct page *, bool);
 
 void hugepage_add_anon_rmap(struct page *, struct vm_area_struct *,
unsigned long);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index cb346f26a22d..5523daf59953 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -183,7 +183,7 @@ static int __replace_page(struct vm_area_struct *vma, 
unsigned long addr,
goto unlock;
 
get_page(kpage);
-   page_add_new_anon_rmap(kpage, vma, addr);
+   page_add_new_anon_rmap(kpage, vma, addr, false);
mem_cgroup_commit_charge(kpage, memcg, false);
lru_cache_add_active_or_unevictable(kpage, vma);
 
@@ -196,7 +196,7 @@ static int __replace_page(struct vm_area_struct *vma, 
unsigned long addr,
ptep_clear_flush_notify(vma, addr, ptep);
set_pte_at_notify(mm, addr, ptep, mk_pte(kpage, vma->vm_page_prot));
 
-   page_remove_rmap(page);
+   page_remove_rmap(page, false);
if (!page_mapped(page))
try_to_free_swap(page);
pte_unmap_unlock(ptep, ptl);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 5f4c97e1a6da..36637a80669e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -743,7 +743,7 @@ static int __do_huge_pmd_anonymous_page(struct mm_struct 
*mm,
pmd_t entry;
entry = mk_huge_pmd(page, vma->vm_page_prot);
entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
-   page_add_new_anon_rmap(page, vma, haddr);
+   page_add_new_anon_rmap(page, vma, haddr, true);
mem_cgroup_commit_charge(page, memcg, false);
lru_cache_add_active_or_unevictable(page, vma);
pgtable_trans_huge_deposit(mm, pmd, pgtable);
@@ -1034,7 +1034,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct 
*mm,
entry = maybe_mkwrite(pte_mkdirty(entry), vma);
memcg = (void *)page_private(pages[i]);
set_page_private(pages[i], 0);
-   page_add_new_anon_rmap(pages[i], vma, haddr);
+   page_add_new_anon_rmap(pages[i], vma, haddr, false);
mem_cgroup_commit_charge(pages[i], memcg, false);
lru_cache_add_active_or_unevictable(pages[i], vma);
pte = pte_offset_map(&_pmd, haddr);
@@ -1046,7 +1046,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct 
*mm,
 
smp_wmb(); /* make pte visible before pmd */
pmd_populate(mm, pmd, pgtable);
-   page_remove_rmap(page);
+   page_remove_rmap(page, true);
spin_unlock(ptl);
 
mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
@@ -1168,7 +1168,7 @@ alloc:
entry = mk_huge_pmd(new_page, vma->vm_page_prot);
entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
pmdp_clear_flush_notify(vma, haddr, pmd);
-   page_add_new_anon_rmap(new_page, vma, haddr);
+   page_add_new_anon_rmap(new_page, vma, haddr, true);

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

2015-02-12 Thread Rik van Riel
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/12/2015 11:18 AM, Kirill A. Shutemov wrote:

 +++ b/include/linux/rmap.h @@ -168,16 +168,24 @@ static inline void
 anon_vma_merge(struct vm_area_struct *vma,
 
 struct anon_vma *page_get_anon_vma(struct page *page);
 
 +/* flags for do_page_add_anon_rmap() */ +enum { +RMAP_EXCLUSIVE =
 1, +  RMAP_COMPOUND = 2, +};

Always a good idea to name things. However, exclusive is
not that clear to me. Given that the argument is supposed
to indicate whether we map a single or a compound page,
maybe the names in the enum could just be SINGLE and COMPOUND?

Naming the enum should make it clear enough what it does:

 enum rmap_page {
  SINGLE = 0,
  COMPOUND
 }

 +++ b/kernel/events/uprobes.c @@ -183,7 +183,7 @@ static int
 __replace_page(struct vm_area_struct *vma, unsigned long addr, goto
 unlock;
 
 get_page(kpage); -page_add_new_anon_rmap(kpage, vma, addr); +
 page_add_new_anon_rmap(kpage, vma, addr, false); 
 mem_cgroup_commit_charge(kpage, memcg, false); 
 lru_cache_add_active_or_unevictable(kpage, vma);

Would it make sense to use the name in the argument to that function,
too?

I often find it a lot easier to see what things do if they use symbolic
names, rather than by trying to remember what each boolean argument to
a function does.

- -- 
All rights reversed
-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQEcBAEBAgAGBQJU3Ra9AAoJEM553pKExN6D4UcH/10GlcYBB813KE7dR2r23MDx
WlrcC096IRoEjD/aaBHikLcKSu5mZDzf3ic1ZHzMPzz7oMdsFkmnY/k2zMdcqc83
7scvd7VB3acI4STKWcbkaCsIHIpHPFmfdcLv9Rabi0P2MBb8SALQCwxDUJqvXojC
JdJivfuagDoSUEamHwZrCvFylC7J7M4zPLD5aUpc93E4I4lhG9VHD7FmnYP3rxb8
kX4DOZFZ7aTN3K9IweCZPN2HWZe2qcSKc/AmIfHfokdjJLTuqbMv5UGSwLHmmeDf
DO4Uru/BMgPg2Ds7uKZosf7icAnOzT08b/Woh34JT83ua9XpFMam+hx6g+lA78E=
=Kzss
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2015-02-12 Thread Kirill A. Shutemov
We're going to allow mapping of individual 4k pages of THP compound
page. It means we cannot rely on PageTransHuge() check to decide if map
small page or THP.

The patch adds new argument to rmap function to indicate whethe we want
to map whole compound page or only the small page.

Signed-off-by: Kirill A. Shutemov kirill.shute...@linux.intel.com
---
 include/linux/rmap.h| 14 +++---
 kernel/events/uprobes.c |  4 ++--
 mm/huge_memory.c| 16 
 mm/hugetlb.c|  4 ++--
 mm/ksm.c|  4 ++--
 mm/memory.c | 14 +++---
 mm/migrate.c|  8 
 mm/rmap.c   | 43 +++
 mm/swapfile.c   |  4 ++--
 9 files changed, 65 insertions(+), 46 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index c4088feac1fc..3bf73620b672 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -168,16 +168,24 @@ static inline void anon_vma_merge(struct vm_area_struct 
*vma,
 
 struct anon_vma *page_get_anon_vma(struct page *page);
 
+/* flags for do_page_add_anon_rmap() */
+enum {
+   RMAP_EXCLUSIVE = 1,
+   RMAP_COMPOUND = 2,
+};
+
 /*
  * rmap interfaces called when adding or removing pte of page
  */
 void page_move_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
long);
-void page_add_anon_rmap(struct page *, struct vm_area_struct *, unsigned long);
+void page_add_anon_rmap(struct page *, struct vm_area_struct *,
+   unsigned long, bool);
 void do_page_add_anon_rmap(struct page *, struct vm_area_struct *,
   unsigned long, int);
-void page_add_new_anon_rmap(struct page *, struct vm_area_struct *, unsigned 
long);
+void page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
+   unsigned long, bool);
 void page_add_file_rmap(struct page *);
-void page_remove_rmap(struct page *);
+void page_remove_rmap(struct page *, bool);
 
 void hugepage_add_anon_rmap(struct page *, struct vm_area_struct *,
unsigned long);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index cb346f26a22d..5523daf59953 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -183,7 +183,7 @@ static int __replace_page(struct vm_area_struct *vma, 
unsigned long addr,
goto unlock;
 
get_page(kpage);
-   page_add_new_anon_rmap(kpage, vma, addr);
+   page_add_new_anon_rmap(kpage, vma, addr, false);
mem_cgroup_commit_charge(kpage, memcg, false);
lru_cache_add_active_or_unevictable(kpage, vma);
 
@@ -196,7 +196,7 @@ static int __replace_page(struct vm_area_struct *vma, 
unsigned long addr,
ptep_clear_flush_notify(vma, addr, ptep);
set_pte_at_notify(mm, addr, ptep, mk_pte(kpage, vma-vm_page_prot));
 
-   page_remove_rmap(page);
+   page_remove_rmap(page, false);
if (!page_mapped(page))
try_to_free_swap(page);
pte_unmap_unlock(ptep, ptl);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 5f4c97e1a6da..36637a80669e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -743,7 +743,7 @@ static int __do_huge_pmd_anonymous_page(struct mm_struct 
*mm,
pmd_t entry;
entry = mk_huge_pmd(page, vma-vm_page_prot);
entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
-   page_add_new_anon_rmap(page, vma, haddr);
+   page_add_new_anon_rmap(page, vma, haddr, true);
mem_cgroup_commit_charge(page, memcg, false);
lru_cache_add_active_or_unevictable(page, vma);
pgtable_trans_huge_deposit(mm, pmd, pgtable);
@@ -1034,7 +1034,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct 
*mm,
entry = maybe_mkwrite(pte_mkdirty(entry), vma);
memcg = (void *)page_private(pages[i]);
set_page_private(pages[i], 0);
-   page_add_new_anon_rmap(pages[i], vma, haddr);
+   page_add_new_anon_rmap(pages[i], vma, haddr, false);
mem_cgroup_commit_charge(pages[i], memcg, false);
lru_cache_add_active_or_unevictable(pages[i], vma);
pte = pte_offset_map(_pmd, haddr);
@@ -1046,7 +1046,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct 
*mm,
 
smp_wmb(); /* make pte visible before pmd */
pmd_populate(mm, pmd, pgtable);
-   page_remove_rmap(page);
+   page_remove_rmap(page, true);
spin_unlock(ptl);
 
mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
@@ -1168,7 +1168,7 @@ alloc:
entry = mk_huge_pmd(new_page, vma-vm_page_prot);
entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
pmdp_clear_flush_notify(vma, haddr, pmd);
-   page_add_new_anon_rmap(new_page, vma, haddr);
+   page_add_new_anon_rmap(new_page, vma, haddr, true);