From: Mel Gorman <mgorman@suse.de>
To: Michal Hocko <mhocko@suse.cz>
Cc: Larry Woodman <lwoodman@redhat.com>,
Rik van Riel <riel@redhat.com>, Hugh Dickins <hughd@google.com>,
Linux-MM <linux-mm@kvack.org>,
David Gibson <david@gibson.dropbear.id.au>,
Ken Chen <kenchen@google.com>,
Cong Wang <xiyou.wangcong@gmail.com>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH -alternative] mm: hugetlbfs: Close race during teardown of hugetlbfs shared page tables V2 (resend)
Date: Thu, 2 Aug 2012 08:37:57 +0100 [thread overview]
Message-ID: <20120802073757.GC29814@suse.de> (raw)
In-Reply-To: <20120802071934.GA7557@dhcp22.suse.cz>
On Thu, Aug 02, 2012 at 09:19:34AM +0200, Michal Hocko wrote:
> Hi Larry,
>
> On Wed 01-08-12 11:06:33, Larry Woodman wrote:
> > On 08/01/2012 08:32 AM, Michal Hocko wrote:
> > >
> > >I am really lame :/. The previous patch is wrong as well for goto out
> > >branch. The updated patch as follows:
> > This patch worked fine Michal!
>
> Thanks for the good news!
>
> > You and Mel can duke it out over who's is best. :)
>
> The answer is clear here ;) Mel did the hard work of identifying the
> culprit so kudos go to him.
I'm happy once it's fixed!
> I just tried to solve the issue more inside x86 arch code. The pmd
> allocation outside of sharing code seemed strange to me for quite some
> time I just underestimated its consequences completely.
>
> Both approaches have some pros. Mel's patch is more resistant to other
> not-yet-discovered races and it also makes the arch independent code
> more robust because relying on the pmd trick is not ideal.
If there is another race then it is best to hear about it, understand
it and fix the underlying problem. More importantly, your patch ensures
that two processes faulting at the same time will share page tables with
each other. My patch only noted that this missed opportunity could cause
problems with fork.
> On the other hand, mine is more coupled with the sharing code so it
> makes the code easier to follow and also makes the sharing more
> effective because racing processes see pmd populated when checking for
> shareable mappings.
>
It could do with a small comment above huge_pmd_share() explaining that
calling pmd_alloc() under the i_mmap_mutex is necessary to prevent two
parallel faults missing a sharing opportunity with each other but it's
not mandatory.
> So I am more inclined to mine but I don't want to push it because both
> are good and make sense. What other people think?
>
I vote yours
Reviewed-by: Mel Gorman <mgorman@suse.de>
--
Mel Gorman
SUSE Labs
--
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>
next prev parent reply other threads:[~2012-08-02 7:38 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-20 13:49 [PATCH] mm: hugetlbfs: Close race during teardown of hugetlbfs shared page tables v2 Mel Gorman
2012-07-20 14:11 ` [PATCH] mm: hugetlbfs: Close race during teardown of hugetlbfs shared page tables V2 (resend) Mel Gorman
2012-07-20 14:29 ` Michal Hocko
2012-07-20 14:37 ` Mel Gorman
2012-07-20 14:40 ` Michal Hocko
2012-07-20 14:36 ` [PATCH -alternative] " Michal Hocko
2012-07-20 14:51 ` Mel Gorman
2012-07-23 4:04 ` Hugh Dickins
2012-07-23 11:40 ` Mel Gorman
2012-07-24 1:08 ` Hugh Dickins
2012-07-24 8:32 ` Michal Hocko
2012-07-24 9:34 ` Mel Gorman
2012-07-24 10:04 ` Michal Hocko
2012-07-24 19:23 ` Hugh Dickins
2012-07-25 8:36 ` Mel Gorman
2012-07-26 17:42 ` Rik van Riel
2012-07-26 18:04 ` Larry Woodman
2012-07-27 8:42 ` Mel Gorman
2012-07-26 18:37 ` Rik van Riel
2012-07-26 21:03 ` Larry Woodman
2012-07-27 3:48 ` Larry Woodman
2012-07-27 10:10 ` Larry Woodman
2012-07-27 10:23 ` Mel Gorman
2012-07-27 10:36 ` Larry Woodman
2012-07-30 19:11 ` Larry Woodman
2012-07-31 12:16 ` Hillf Danton
2012-07-31 12:46 ` Mel Gorman
2012-07-31 13:07 ` Larry Woodman
2012-07-31 13:29 ` Mel Gorman
2012-07-31 13:21 ` Michal Hocko
2012-07-31 17:49 ` Larry Woodman
2012-07-31 20:06 ` Michal Hocko
2012-07-31 20:57 ` Larry Woodman
2012-08-01 2:45 ` Larry Woodman
2012-08-01 8:20 ` Michal Hocko
2012-08-01 12:32 ` Michal Hocko
2012-08-01 15:06 ` Larry Woodman
2012-08-02 7:19 ` Michal Hocko
2012-08-02 7:37 ` Mel Gorman [this message]
2012-08-02 12:36 ` Michal Hocko
2012-08-02 13:33 ` Mel Gorman
2012-08-02 13:53 ` Michal Hocko
2012-07-31 18:03 ` Rik van Riel
2012-07-26 18:31 ` Rik van Riel
2012-07-27 9:02 ` Michal Hocko
2012-07-26 16:01 ` [PATCH] mm: hugetlbfs: Close race during teardown of hugetlbfs shared page tables v2 Larry Woodman
2012-07-27 8:47 ` Mel Gorman
2012-07-26 21:00 ` Rik van Riel
2012-07-26 21:54 ` Hugh Dickins
2012-07-27 8:52 ` Mel Gorman
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=20120802073757.GC29814@suse.de \
--to=mgorman@suse.de \
--cc=david@gibson.dropbear.id.au \
--cc=hughd@google.com \
--cc=kenchen@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lwoodman@redhat.com \
--cc=mhocko@suse.cz \
--cc=riel@redhat.com \
--cc=xiyou.wangcong@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).