linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Luiz Capitulino <lcapitulino@redhat.com>,
	Michal Nazarewicz <mina86@mina86.com>,
	Michal Hocko <mhocko@kernel.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Mel Gorman <mgorman@techsingularity.net>,
	Johannes Weiner <hannes@cmpxchg.org>
Subject: Re: [PATCH 1/1] mm: make start_isolate_page_range() fail if already isolated
Date: Tue, 6 Mar 2018 14:32:51 -0800	[thread overview]
Message-ID: <20180306143251.f83d992aaee64fb3c1a1993c@linux-foundation.org> (raw)
In-Reply-To: <0c473e9c-d28b-b965-2f14-5d195e404d0c@oracle.com>

On Mon, 5 Mar 2018 16:57:40 -0800 Mike Kravetz <mike.kravetz@oracle.com> wrote:

> >>>
> >>> I could not immediately come up with a good idea here.  The zone lock
> >>> would be the obvious choice, but I don't think we want to hold it while
> >>> examining each of the page blocks.  Perhaps a new lock or semaphore
> >>> associated with the zone?  I'm open to suggestions.
> >>
> >> Yes, I think it would need a new lock.  Hopefully a mutex.
> > 
> > I'll look into adding an 'isolate' mutex to the zone structure and reworking
> > this patch.
> 
> I went back and examined the 'isolation functionality' with an eye on perhaps
> adding a mutex for some higher level synchronization.  However, there does
> not appear to be a straight forward solution.
> 
> What we really need is some way of preventing two threads from operating on
> the same set of page blocks concurrently.  We do not want a big mutex, as
> we do want two threads to run in parallel if operating on separate
> non-overlapping ranges (CMA does this today).  If we did this, I think we
> would need a new data structure to represent page blocks within a zone.
> start_isolate_page_range() would then then check the new data structure for
> conflicts, and if none found mark the range it is operating on as 'in use'.
> undo_isolate_page_range() would clear the entries for the range in the new
> data structure.  Such information would hang off the zone and be protected
> by the zone lock.  The new data structure could be static (like a bit map),
> or dynamic.  It certainly is doable, but ...
> 
> The more I think about it, the more I like my original proposal.  The
> comment "blundering through a whole bunch of pages then saying whoops
> then undoing everything is unpleasing" is certainly true.  But do note
> that after isolating the page blocks, we will then attempt to migrate
> pages within those blocks.  There is a more than a minimal chance that
> we will not be able to migrate something within the set of page blocks.
> In that case we again say whoops and undo even more work.
> 
> I am relatively new to this area of code.  Therefore, it would be good to
> get comments from some of the original authors.

hm, OK.  Perhaps it would help to produce a v2 which has more comments
and changelogging describing what's happening here and why things are
as they are.

--
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>

  reply	other threads:[~2018-03-06 22:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-26 19:10 [PATCH 0/1] make start_isolate_page_range() thread safe Mike Kravetz
2018-02-26 19:10 ` [PATCH 1/1] mm: make start_isolate_page_range() fail if already isolated Mike Kravetz
2018-03-03  0:06   ` Andrew Morton
2018-03-03  0:38     ` Mike Kravetz
2018-03-03  0:56       ` Andrew Morton
2018-03-03  1:39         ` Mike Kravetz
2018-03-06  0:57           ` Mike Kravetz
2018-03-06 22:32             ` Andrew Morton [this message]
2018-03-09 22:47 ` [PATCH v2] " Mike Kravetz
2018-03-13 21:14   ` Andrew Morton
2018-03-13 21:27     ` Mike Kravetz

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=20180306143251.f83d992aaee64fb3c1a1993c@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=lcapitulino@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=mike.kravetz@oracle.com \
    --cc=mina86@mina86.com \
    --cc=vbabka@suse.cz \
    /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).