linux-kernel - Re: [PATCH 1/1] mm: make start_isolate_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 6 Mar 2018 14:32:51 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
        Luiz Capitulino <lcapitulino@...hat.com>,
        Michal Nazarewicz <mina86@...a86.com>,
        Michal Hocko <mhocko@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Johannes Weiner <hannes@...xchg.org>
Subject: Re: [PATCH 1/1] mm: make start_isolate_page_range() fail if already
 isolated

On Mon, 5 Mar 2018 16:57:40 -0800 Mike Kravetz <mike.kravetz@...cle.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.