All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Hillf Danton <dhillf@gmail.com>, Mel Gorman <mel@csn.ul.ie>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Rientjes <rientjes@google.com>
Subject: Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
Date: Wed, 26 Sep 2012 16:56:17 -0400	[thread overview]
Message-ID: <20120926205617.GA2667@cmpxchg.org> (raw)
In-Reply-To: <1344866141-27906-1-git-send-email-mhocko@suse.cz>

On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> from vma) fixed pgoff calculation but it has replaced it by
> vma_hugecache_offset which is not approapriate for offsets used for
> vma_prio_tree_foreach because that one expects index in page units
> rather than in huge_page_shift.
> Using vma_hugecache_offset is not incorrect because the pgoff will fit
> into the same vmas but it is confusing so the standard PAGE_SHIFT based
> index calculation is used instead.

I do think it's incorrect.  The resulting index may not be too big,
but it can be too small: assume hpage size of 2M and the address to
unmap to be 0x200000.  This is regular page index 512 and hpage index
1.  If you have a VMA that maps the file only starting at the second
huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
miss it even though it does map the page of interest.  hugetlb_cow()
will try to unmap, miss the vma, and retry the cow until the
allocation succeeds or the skipped vma(s) go away.

Unless I missed something, this should not be deferred as a cleanup.

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Hillf Danton <dhillf@gmail.com>, Mel Gorman <mel@csn.ul.ie>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Rientjes <rientjes@google.com>
Subject: Re: [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach
Date: Wed, 26 Sep 2012 16:56:17 -0400	[thread overview]
Message-ID: <20120926205617.GA2667@cmpxchg.org> (raw)
In-Reply-To: <1344866141-27906-1-git-send-email-mhocko@suse.cz>

On Mon, Aug 13, 2012 at 03:55:41PM +0200, Michal Hocko wrote:
> 0c176d5 (mm: hugetlb: fix pgoff computation when unmapping page
> from vma) fixed pgoff calculation but it has replaced it by
> vma_hugecache_offset which is not approapriate for offsets used for
> vma_prio_tree_foreach because that one expects index in page units
> rather than in huge_page_shift.
> Using vma_hugecache_offset is not incorrect because the pgoff will fit
> into the same vmas but it is confusing so the standard PAGE_SHIFT based
> index calculation is used instead.

I do think it's incorrect.  The resulting index may not be too big,
but it can be too small: assume hpage size of 2M and the address to
unmap to be 0x200000.  This is regular page index 512 and hpage index
1.  If you have a VMA that maps the file only starting at the second
huge page, that VMAs vm_pgoff will be 512 but you ask for offset 1 and
miss it even though it does map the page of interest.  hugetlb_cow()
will try to unmap, miss the vma, and retry the cow until the
allocation succeeds or the skipped vma(s) go away.

Unless I missed something, this should not be deferred as a cleanup.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2012-09-26 20:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-13 13:55 [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach Michal Hocko
2012-08-13 13:55 ` Michal Hocko
2012-08-16 12:45 ` Hillf Danton
2012-08-16 12:45   ` Hillf Danton
2012-08-16 15:11   ` Michal Hocko
2012-08-16 15:11     ` Michal Hocko
2012-09-26 20:56 ` Johannes Weiner [this message]
2012-09-26 20:56   ` Johannes Weiner
2012-10-01 16:22   ` Michal Hocko
2012-10-01 16:22     ` Michal Hocko
2012-10-01 18:29     ` Johannes Weiner
2012-10-01 18:29       ` Johannes Weiner
  -- strict thread matches above, loose matches on Subject: below --
2012-08-13 13:09 [patch] hugetlb: correct page offset index for sharing pmd Michal Hocko
2012-08-13 13:24 ` [PATCH] hugetlb: do not use vma_hugecache_offset for vma_prio_tree_foreach Hillf Danton
2012-08-13 13:24   ` Hillf Danton
2012-08-13 13:49   ` Michal Hocko
2012-08-13 13:49     ` Michal Hocko
2012-08-13 13:51     ` Hillf Danton
2012-08-13 13:51       ` Hillf Danton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120926205617.GA2667@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dhillf@gmail.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mel@csn.ul.ie \
    --cc=mhocko@suse.cz \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.