On Mon, Feb 01, 2016 at 11:28:54AM +0530, Anshuman Khandual wrote:

On 01/29/2016 10:53 AM, David Gibson wrote:

quoted

At the moment the hpte_removebolted callback in ppc_md returns void and
will BUG_ON() if the hpte it's asked to remove doesn't exist in the first
place.  This is awkward for the case of cleaning up a mapping which was
partially made before failing.

So, we add a return value to hpte_removebolted, and have it return ENOENT
in the case that the HPTE to remove didn't exist in the first place.

In the (sole) caller, we propagate errors in hpte_removebolted to its
caller to handle.  However, we handle ENOENT specially, continuing to
complete the unmapping over the specified range before returning the error
to the caller.

This means that htab_remove_mapping() will work sanely on a partially
present mapping, removing any HPTEs which are present, while also returning
ENOENT to its caller in case it's important there.

Yeah makes sense.

quoted

There are two callers of htab_remove_mapping():
   - In remove_section_mapping() we already WARN_ON() any error return,
     which is reasonable - in this case the mapping should be fully
     present

Right.

quoted

   - In vmemmap_remove_mapping() we BUG_ON() any error.  We change that to
     just a WARN_ON() in the case of ENOENT, since failing to remove a
     mapping that wasn't there in the first place probably shouldn't be
     fatal.

Provided the caller of vmemmap_remove_mapping() which is memory hotplug
path must be handling the returned -ENOENT error correctly.

vmemmap_remove_mapping() is void, so there's no -ENOENT returned, just
the WARN_ON().

Just curious
and want to make sure that any of the memory sections or pages inside the
section must not be left in a state which makes the next call in the
hotplug path fail.

So, this situation shouldn't happen - the mapping should be complete -
but there's nothing obvious that the caller should do extra.  It asked
that the mapping be removed, and we discovered that some of it wasn't
there to begin with.  Whether we can continue safely depends on
what exactly caused the mapping not to be fully present in the first
place, and whether that had other conseuqences, but we have no way of
knowing that here.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson